Skip to content

Commit e107aae

Browse files
authored
Update Monitoring plugin's Elasticsearch configuration (#55119) (#55435)
* Fix Monitoring plugin Elasticsearch SSL config Plugin now allows "keystore" and "truststore" values in its config schema as the documentation currently states. Plugin also now reads PEM and PKCS12 files off of the filesystem before attempting to create an Elasticsearch client. * Add missing Elasticsearch config deprecations Several Elasticsearch config deprecations were overlooked for monitoring-specific Elasticsearch settings.
1 parent 0be6024 commit e107aae

File tree

9 files changed

+408
-20
lines changed

9 files changed

+408
-20
lines changed

x-pack/legacy/plugins/monitoring/__tests__/deprecations.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,54 @@ describe('monitoring plugin deprecations', function() {
8080
expect(log.called).to.be(true);
8181
});
8282
});
83+
84+
describe('elasticsearch.username', function() {
85+
it('logs a warning if elasticsearch.username is set to "elastic"', () => {
86+
const settings = { elasticsearch: { username: 'elastic' } };
87+
88+
const log = sinon.spy();
89+
transformDeprecations(settings, log);
90+
expect(log.called).to.be(true);
91+
});
92+
93+
it('does not log a warning if elasticsearch.username is set to something besides "elastic"', () => {
94+
const settings = { elasticsearch: { username: 'otheruser' } };
95+
96+
const log = sinon.spy();
97+
transformDeprecations(settings, log);
98+
expect(log.called).to.be(false);
99+
});
100+
101+
it('does not log a warning if elasticsearch.username is unset', () => {
102+
const settings = { elasticsearch: { username: undefined } };
103+
104+
const log = sinon.spy();
105+
transformDeprecations(settings, log);
106+
expect(log.called).to.be(false);
107+
});
108+
109+
it('logs a warning if ssl.key is set and ssl.certificate is not', () => {
110+
const settings = { elasticsearch: { ssl: { key: '' } } };
111+
112+
const log = sinon.spy();
113+
transformDeprecations(settings, log);
114+
expect(log.called).to.be(true);
115+
});
116+
117+
it('logs a warning if ssl.certificate is set and ssl.key is not', () => {
118+
const settings = { elasticsearch: { ssl: { certificate: '' } } };
119+
120+
const log = sinon.spy();
121+
transformDeprecations(settings, log);
122+
expect(log.called).to.be(true);
123+
});
124+
125+
it('does not log a warning if both ssl.key and ssl.certificate are set', () => {
126+
const settings = { elasticsearch: { ssl: { key: '', certificate: '' } } };
127+
128+
const log = sinon.spy();
129+
transformDeprecations(settings, log);
130+
expect(log.called).to.be(false);
131+
});
132+
});
83133
});

x-pack/legacy/plugins/monitoring/config.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,14 @@ export const config = Joi => {
8383
certificate: Joi.string(),
8484
key: Joi.string(),
8585
keyPassphrase: Joi.string(),
86+
keystore: Joi.object({
87+
path: Joi.string(),
88+
password: Joi.string(),
89+
}).default(),
90+
truststore: Joi.object({
91+
path: Joi.string(),
92+
password: Joi.string(),
93+
}).default(),
8694
alwaysPresentCertificate: Joi.boolean().default(false),
8795
}).default(),
8896
apiVersion: Joi.string().default('master'),

x-pack/legacy/plugins/monitoring/deprecations.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,31 @@ export const deprecations = () => {
2727
);
2828
}
2929
},
30+
(settings, log) => {
31+
const fromPath = 'xpack.monitoring.elasticsearch';
32+
const es = get(settings, 'elasticsearch');
33+
if (es) {
34+
if (es.username === 'elastic') {
35+
log(
36+
`Setting [${fromPath}.username] to "elastic" is deprecated. You should use the "kibana" user instead.`
37+
);
38+
}
39+
}
40+
},
41+
(settings, log) => {
42+
const fromPath = 'xpack.monitoring.elasticsearch.ssl';
43+
const ssl = get(settings, 'elasticsearch.ssl');
44+
if (ssl) {
45+
if (ssl.key !== undefined && ssl.certificate === undefined) {
46+
log(
47+
`Setting [${fromPath}.key] without [${fromPath}.certificate] is deprecated. This has no effect, you should use both settings to enable TLS client authentication to Elasticsearch.`
48+
);
49+
} else if (ssl.certificate !== undefined && ssl.key === undefined) {
50+
log(
51+
`Setting [${fromPath}.certificate] without [${fromPath}.key] is deprecated. This has no effect, you should use both settings to enable TLS client authentication to Elasticsearch.`
52+
);
53+
}
54+
}
55+
},
3056
];
3157
};

x-pack/legacy/plugins/monitoring/server/es_client/__tests__/instantiate_client.js

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import expect from '@kbn/expect';
88
import sinon from 'sinon';
9-
import { get, noop } from 'lodash';
9+
import { noop } from 'lodash';
1010
import { exposeClient, hasMonitoringCluster } from '../instantiate_client';
1111

1212
function getMockServerFromConnectionUrl(monitoringClusterUrl) {
@@ -26,15 +26,8 @@ function getMockServerFromConnectionUrl(monitoringClusterUrl) {
2626
},
2727
};
2828

29-
const config = {
30-
get: path => {
31-
return get(server, path);
32-
},
33-
set: noop,
34-
};
35-
3629
return {
37-
config,
30+
elasticsearchConfig: server.xpack.monitoring.elasticsearch,
3831
elasticsearchPlugin: {
3932
getCluster: sinon
4033
.stub()
@@ -141,12 +134,12 @@ describe('Instantiate Client', () => {
141134
describe('hasMonitoringCluster', () => {
142135
it('returns true if monitoring is configured', () => {
143136
const server = getMockServerFromConnectionUrl('http://monitoring-cluster.test:9200'); // pass null for URL to create the client using prod config
144-
expect(hasMonitoringCluster(server.config)).to.be(true);
137+
expect(hasMonitoringCluster(server.elasticsearchConfig)).to.be(true);
145138
});
146139

147140
it('returns false if monitoring is not configured', () => {
148141
const server = getMockServerFromConnectionUrl(null);
149-
expect(hasMonitoringCluster(server.config)).to.be(false);
142+
expect(hasMonitoringCluster(server.elasticsearchConfig)).to.be(false);
150143
});
151144
});
152145
});

x-pack/legacy/plugins/monitoring/server/es_client/instantiate_client.js

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,24 +14,21 @@ import { LOGGING_TAG } from '../../common/constants';
1414
* Kibana itself is connected to a production cluster.
1515
*/
1616

17-
export function exposeClient({ config, events, log, elasticsearchPlugin }) {
18-
const elasticsearchConfig = hasMonitoringCluster(config)
19-
? config.get('xpack.monitoring.elasticsearch')
20-
: {};
17+
export function exposeClient({ elasticsearchConfig, events, log, elasticsearchPlugin }) {
18+
const isMonitoringCluster = hasMonitoringCluster(elasticsearchConfig);
2119
const cluster = elasticsearchPlugin.createCluster('monitoring', {
22-
...elasticsearchConfig,
20+
...(isMonitoringCluster ? elasticsearchConfig : {}),
2321
plugins: [monitoringBulk],
2422
logQueries: Boolean(elasticsearchConfig.logQueries),
2523
});
2624

2725
events.on('stop', bindKey(cluster, 'close'));
28-
const configSource = hasMonitoringCluster(config) ? 'monitoring' : 'production';
26+
const configSource = isMonitoringCluster ? 'monitoring' : 'production';
2927
log([LOGGING_TAG, 'es-client'], `config sourced from: ${configSource} cluster`);
3028
}
3129

3230
export function hasMonitoringCluster(config) {
33-
const hosts = config.get('xpack.monitoring.elasticsearch.hosts');
34-
return Boolean(hosts && hosts.length);
31+
return Boolean(config.hosts && config.hosts.length);
3532
}
3633

3734
export const instantiateClient = once(exposeClient);
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
export const mockReadFileSync = jest.fn();
8+
jest.mock('fs', () => ({ readFileSync: mockReadFileSync }));
9+
10+
export const mockReadPkcs12Keystore = jest.fn();
11+
export const mockReadPkcs12Truststore = jest.fn();
12+
jest.mock('../../../../../../src/core/utils', () => ({
13+
readPkcs12Keystore: mockReadPkcs12Keystore,
14+
readPkcs12Truststore: mockReadPkcs12Truststore,
15+
}));
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import {
8+
mockReadFileSync,
9+
mockReadPkcs12Keystore,
10+
mockReadPkcs12Truststore,
11+
} from './parse_elasticsearch_config.test.mocks';
12+
13+
import { parseElasticsearchConfig } from './parse_elasticsearch_config';
14+
15+
const parse = (config: any) => {
16+
return parseElasticsearchConfig({
17+
get: () => config,
18+
});
19+
};
20+
21+
describe('reads files', () => {
22+
beforeEach(() => {
23+
mockReadFileSync.mockReset();
24+
mockReadFileSync.mockImplementation((path: string) => `content-of-${path}`);
25+
mockReadPkcs12Keystore.mockReset();
26+
mockReadPkcs12Keystore.mockImplementation((path: string) => ({
27+
key: `content-of-${path}.key`,
28+
cert: `content-of-${path}.cert`,
29+
ca: [`content-of-${path}.ca`],
30+
}));
31+
mockReadPkcs12Truststore.mockReset();
32+
mockReadPkcs12Truststore.mockImplementation((path: string) => [`content-of-${path}`]);
33+
});
34+
35+
it('reads certificate authorities when ssl.keystore.path is specified', () => {
36+
const configValue = parse({ ssl: { keystore: { path: 'some-path' } } });
37+
expect(mockReadPkcs12Keystore).toHaveBeenCalledTimes(1);
38+
expect(configValue.ssl.certificateAuthorities).toEqual(['content-of-some-path.ca']);
39+
});
40+
41+
it('reads certificate authorities when ssl.truststore.path is specified', () => {
42+
const configValue = parse({ ssl: { truststore: { path: 'some-path' } } });
43+
expect(mockReadPkcs12Truststore).toHaveBeenCalledTimes(1);
44+
expect(configValue.ssl.certificateAuthorities).toEqual(['content-of-some-path']);
45+
});
46+
47+
it('reads certificate authorities when ssl.certificateAuthorities is specified', () => {
48+
let configValue = parse({ ssl: { certificateAuthorities: 'some-path' } });
49+
expect(mockReadFileSync).toHaveBeenCalledTimes(1);
50+
expect(configValue.ssl.certificateAuthorities).toEqual(['content-of-some-path']);
51+
52+
mockReadFileSync.mockClear();
53+
configValue = parse({ ssl: { certificateAuthorities: ['some-path'] } });
54+
expect(mockReadFileSync).toHaveBeenCalledTimes(1);
55+
expect(configValue.ssl.certificateAuthorities).toEqual(['content-of-some-path']);
56+
57+
mockReadFileSync.mockClear();
58+
configValue = parse({ ssl: { certificateAuthorities: ['some-path', 'another-path'] } });
59+
expect(mockReadFileSync).toHaveBeenCalledTimes(2);
60+
expect(configValue.ssl.certificateAuthorities).toEqual([
61+
'content-of-some-path',
62+
'content-of-another-path',
63+
]);
64+
});
65+
66+
it('reads certificate authorities when ssl.keystore.path, ssl.truststore.path, and ssl.certificateAuthorities are specified', () => {
67+
const configValue = parse({
68+
ssl: {
69+
keystore: { path: 'some-path' },
70+
truststore: { path: 'another-path' },
71+
certificateAuthorities: 'yet-another-path',
72+
},
73+
});
74+
expect(mockReadPkcs12Keystore).toHaveBeenCalledTimes(1);
75+
expect(mockReadPkcs12Truststore).toHaveBeenCalledTimes(1);
76+
expect(mockReadFileSync).toHaveBeenCalledTimes(1);
77+
expect(configValue.ssl.certificateAuthorities).toEqual([
78+
'content-of-some-path.ca',
79+
'content-of-another-path',
80+
'content-of-yet-another-path',
81+
]);
82+
});
83+
84+
it('reads a private key and certificate when ssl.keystore.path is specified', () => {
85+
const configValue = parse({ ssl: { keystore: { path: 'some-path' } } });
86+
expect(mockReadPkcs12Keystore).toHaveBeenCalledTimes(1);
87+
expect(configValue.ssl.key).toEqual('content-of-some-path.key');
88+
expect(configValue.ssl.certificate).toEqual('content-of-some-path.cert');
89+
});
90+
91+
it('reads a private key when ssl.key is specified', () => {
92+
const configValue = parse({ ssl: { key: 'some-path' } });
93+
expect(mockReadFileSync).toHaveBeenCalledTimes(1);
94+
expect(configValue.ssl.key).toEqual('content-of-some-path');
95+
});
96+
97+
it('reads a certificate when ssl.certificate is specified', () => {
98+
const configValue = parse({ ssl: { certificate: 'some-path' } });
99+
expect(mockReadFileSync).toHaveBeenCalledTimes(1);
100+
expect(configValue.ssl.certificate).toEqual('content-of-some-path');
101+
});
102+
});
103+
104+
describe('throws when config is invalid', () => {
105+
beforeAll(() => {
106+
const realFs = jest.requireActual('fs');
107+
mockReadFileSync.mockImplementation((path: string) => realFs.readFileSync(path));
108+
const utils = jest.requireActual('../../../../../../src/core/utils');
109+
mockReadPkcs12Keystore.mockImplementation((path: string, password?: string) =>
110+
utils.readPkcs12Keystore(path, password)
111+
);
112+
mockReadPkcs12Truststore.mockImplementation((path: string, password?: string) =>
113+
utils.readPkcs12Truststore(path, password)
114+
);
115+
});
116+
117+
it('throws if key is invalid', () => {
118+
const value = { ssl: { key: '/invalid/key' } };
119+
expect(() => parse(value)).toThrowErrorMatchingInlineSnapshot(
120+
`"ENOENT: no such file or directory, open '/invalid/key'"`
121+
);
122+
});
123+
124+
it('throws if certificate is invalid', () => {
125+
const value = { ssl: { certificate: '/invalid/cert' } };
126+
expect(() => parse(value)).toThrowErrorMatchingInlineSnapshot(
127+
`"ENOENT: no such file or directory, open '/invalid/cert'"`
128+
);
129+
});
130+
131+
it('throws if certificateAuthorities is invalid', () => {
132+
const value = { ssl: { certificateAuthorities: '/invalid/ca' } };
133+
expect(() => parse(value)).toThrowErrorMatchingInlineSnapshot(
134+
`"ENOENT: no such file or directory, open '/invalid/ca'"`
135+
);
136+
});
137+
138+
it('throws if keystore path is invalid', () => {
139+
const value = { ssl: { keystore: { path: '/invalid/keystore' } } };
140+
expect(() => parse(value)).toThrowErrorMatchingInlineSnapshot(
141+
`"ENOENT: no such file or directory, open '/invalid/keystore'"`
142+
);
143+
});
144+
145+
it('throws if keystore does not contain a key', () => {
146+
mockReadPkcs12Keystore.mockReturnValueOnce({});
147+
const value = { ssl: { keystore: { path: 'some-path' } } };
148+
expect(() => parse(value)).toThrowErrorMatchingInlineSnapshot(
149+
`"Did not find key in Elasticsearch keystore."`
150+
);
151+
});
152+
153+
it('throws if keystore does not contain a certificate', () => {
154+
mockReadPkcs12Keystore.mockReturnValueOnce({ key: 'foo' });
155+
const value = { ssl: { keystore: { path: 'some-path' } } };
156+
expect(() => parse(value)).toThrowErrorMatchingInlineSnapshot(
157+
`"Did not find certificate in Elasticsearch keystore."`
158+
);
159+
});
160+
161+
it('throws if truststore path is invalid', () => {
162+
const value = { ssl: { keystore: { path: '/invalid/truststore' } } };
163+
expect(() => parse(value)).toThrowErrorMatchingInlineSnapshot(
164+
`"ENOENT: no such file or directory, open '/invalid/truststore'"`
165+
);
166+
});
167+
168+
it('throws if key and keystore.path are both specified', () => {
169+
const value = { ssl: { key: 'foo', keystore: { path: 'bar' } } };
170+
expect(() => parse(value)).toThrowErrorMatchingInlineSnapshot(
171+
`"[config validation of [xpack.monitoring.elasticsearch].ssl]: cannot use [key] when [keystore.path] is specified"`
172+
);
173+
});
174+
175+
it('throws if certificate and keystore.path are both specified', () => {
176+
const value = { ssl: { certificate: 'foo', keystore: { path: 'bar' } } };
177+
expect(() => parse(value)).toThrowErrorMatchingInlineSnapshot(
178+
`"[config validation of [xpack.monitoring.elasticsearch].ssl]: cannot use [certificate] when [keystore.path] is specified"`
179+
);
180+
});
181+
});

0 commit comments

Comments
 (0)