Skip to content

Commit

Permalink
[Reporting] Update some runtime validations (#53975) (#54371)
Browse files Browse the repository at this point in the history
* [Reporting] Update some runtime validations

* fix unit test

* i18n

* make warning logging of encryptionKey possible

* update snapshot

* revert unrelated config change
  • Loading branch information
tsullivan authored Jan 9, 2020
1 parent 05d0293 commit 5fedfc5
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 12 deletions.
8 changes: 8 additions & 0 deletions docs/settings/reporting-settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ The protocol for accessing Kibana, typically `http` or `https`.
`xpack.reporting.kibanaServer.hostname`::
The hostname for accessing {kib}, if different from the `server.host` value.

[NOTE]
============
Reporting authenticates requests on the Kibana page only when the hostname matches the
`xpack.reporting.kibanaServer.hostname` setting. Therefore Reporting would fail if the
set value redirects to another server. For that reason, `"0"` is an invalid setting
because, in the Reporting browser, it becomes an automatic redirect to `"0.0.0.0"`.
============


[float]
[[reporting-job-queue-settings]]
Expand Down
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/reporting/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export async function config(Joi: any) {
enabled: Joi.boolean().default(true),
kibanaServer: Joi.object({
protocol: Joi.string().valid(['http', 'https']),
hostname: Joi.string(),
hostname: Joi.string().invalid('0'),
port: Joi.number().integer(),
}).default(),
queue: Joi.object({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@

import expect from '@kbn/expect';
import sinon from 'sinon';
import { validateConfig } from '../validate_config';
import { validateEncryptionKey } from '../validate_encryption_key';

// FAILING: https://github.com/elastic/kibana/issues/51373
describe.skip('Reporting: Validate config', () => {
describe('Reporting: Validate config', () => {
const logger = {
warning: sinon.spy(),
};
Expand All @@ -25,7 +24,7 @@ describe.skip('Reporting: Validate config', () => {
set: sinon.stub(),
};

expect(() => validateConfig(config, logger)).not.to.throwError();
expect(() => validateEncryptionKey({ config: () => config }, logger)).not.to.throwError();

sinon.assert.calledWith(config.set, 'xpack.reporting.encryptionKey');
sinon.assert.calledWithMatch(logger.warning, /Generating a random key/);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import expect from '@kbn/expect';
import sinon from 'sinon';
import { ServerFacade } from '../../../../types';
import { validateServerHost } from '../validate_server_host';

const configKey = 'xpack.reporting.kibanaServer.hostname';

describe('Reporting: Validate server host setting', () => {
it(`should log a warning and set ${configKey} if server.host is "0"`, () => {
const getStub = sinon.stub();
getStub.withArgs('server.host').returns('0');
getStub.withArgs(configKey).returns(undefined);
const config = {
get: getStub,
set: sinon.stub(),
};

expect(() =>
validateServerHost(({ config: () => config } as unknown) as ServerFacade)
).to.throwError();

sinon.assert.calledWith(config.set, configKey);
});
});
20 changes: 16 additions & 4 deletions x-pack/legacy/plugins/reporting/server/lib/validate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { i18n } from '@kbn/i18n';
import { ServerFacade, Logger } from '../../../types';
import { HeadlessChromiumDriverFactory } from '../../browsers/chromium/driver_factory';
import { validateBrowser } from './validate_browser';
import { validateConfig } from './validate_config';
import { validateEncryptionKey } from './validate_encryption_key';
import { validateMaxContentLength } from './validate_max_content_length';
import { validateServerHost } from './validate_server_host';

export async function runValidations(
server: ServerFacade,
Expand All @@ -18,13 +20,23 @@ export async function runValidations(
try {
await Promise.all([
validateBrowser(server, browserFactory, logger),
validateConfig(server, logger),
validateEncryptionKey(server, logger),
validateMaxContentLength(server, logger),
validateServerHost(server),
]);
logger.debug(`Reporting plugin self-check ok!`);
logger.debug(
i18n.translate('xpack.reporting.selfCheck.ok', {
defaultMessage: `Reporting plugin self-check ok!`,
})
);
} catch (err) {
logger.warning(
`Reporting plugin self-check failed. Please check the Kibana Reporting settings. ${err}`
i18n.translate('xpack.reporting.selfCheck.warning', {
defaultMessage: `Reporting plugin self-check generated a warning: {err}`,
values: {
err,
},
})
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,25 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { i18n } from '@kbn/i18n';
import crypto from 'crypto';
import { ServerFacade, Logger } from '../../../types';

export function validateConfig(serverFacade: ServerFacade, logger: Logger) {
export function validateEncryptionKey(serverFacade: ServerFacade, logger: Logger) {
const config = serverFacade.config();

const encryptionKey = config.get('xpack.reporting.encryptionKey');
if (encryptionKey == null) {
// TODO this should simply throw an error and let the handler conver it to a warning mesasge. See validateServerHost.
logger.warning(
`Generating a random key for xpack.reporting.encryptionKey. To prevent pending reports from failing on restart, please set ` +
`xpack.reporting.encryptionKey in kibana.yml`
i18n.translate('xpack.reporting.selfCheckEncryptionKey.warning', {
defaultMessage:
`Generating a random key for {setting}. To prevent pending reports ` +
`from failing on restart, please set {setting} in kibana.yml`,
values: {
setting: 'xpack.reporting.encryptionKey',
},
})
);

// @ts-ignore: No set() method on KibanaConfig, just get() and has()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export async function validateMaxContentLength(server: ServerFacade, logger: Log
const kibanaMaxContentBytes: number = config.get(KIBANA_MAX_SIZE_BYTES_PATH);

if (kibanaMaxContentBytes > elasticSearchMaxContentBytes) {
// TODO this should simply throw an error and let the handler conver it to a warning mesasge. See validateServerHost.
logger.warning(
`${KIBANA_MAX_SIZE_BYTES_PATH} (${kibanaMaxContentBytes}) is higher than ElasticSearch's ${ES_MAX_SIZE_BYTES_PATH} (${elasticSearchMaxContentBytes}). ` +
`Please set ${ES_MAX_SIZE_BYTES_PATH} in ElasticSearch to match, or lower your ${KIBANA_MAX_SIZE_BYTES_PATH} in Kibana to avoid this warning.`
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { ServerFacade } from '../../../types';

const configKey = 'xpack.reporting.kibanaServer.hostname';

export function validateServerHost(serverFacade: ServerFacade) {
const config = serverFacade.config();

const serverHost = config.get('server.host');
const reportingKibanaHostName = config.get(configKey);

if (!reportingKibanaHostName && serverHost === '0') {
// @ts-ignore: No set() method on KibanaConfig, just get() and has()
config.set(configKey, '0.0.0.0'); // update config in memory to allow Reporting to work

throw new Error(
`Found 'server.host: "0"' in settings. This is incompatible with Reporting. ` +
`To enable Reporting to work, '${configKey}: 0.0.0.0' is being automatically to the configuration. ` +
`You can change to 'server.host: 0.0.0.0' or add '${configKey}: 0.0.0.0' in kibana.yml to prevent this message.`
);
}
}

0 comments on commit 5fedfc5

Please sign in to comment.