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

[Reporting] Convert plugin setup and start to synchronous #68326

Merged
merged 13 commits into from
Jun 11, 2020
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,25 @@ import { getChromeLogLocation } from '../paths';
import { puppeteerLaunch } from '../puppeteer';
import { args } from './args';

type binaryPath = string;
type BrowserConfig = CaptureConfig['browser']['chromium'];
type ViewportConfig = CaptureConfig['viewport'];

export class HeadlessChromiumDriverFactory {
private binaryPath: binaryPath;
private binaryPath: string;
private captureConfig: CaptureConfig;
private browserConfig: BrowserConfig;
private userDataDir: string;
private getChromiumArgs: (viewport: ViewportConfig) => string[];

constructor(binaryPath: binaryPath, logger: LevelLogger, captureConfig: CaptureConfig) {
constructor(binaryPath: string, captureConfig: CaptureConfig, logger: LevelLogger) {
this.binaryPath = binaryPath;
this.captureConfig = captureConfig;
this.browserConfig = captureConfig.browser.chromium;

if (this.browserConfig.disableSandbox) {
logger.warning(`Enabling the Chromium sandbox provides an additional layer of protection.`);
}

this.userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'chromium-'));
this.getChromiumArgs = (viewport: ViewportConfig) =>
args({
Expand Down
16 changes: 7 additions & 9 deletions x-pack/plugins/reporting/server/browsers/chromium/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { BrowserDownload } from '../';
import { CaptureConfig } from '../../../server/types';
import { LevelLogger } from '../../lib';
import { HeadlessChromiumDriverFactory } from './driver_factory';
import { paths } from './paths';

export { paths } from './paths';

export async function createDriverFactory(
binaryPath: string,
logger: LevelLogger,
captureConfig: CaptureConfig
): Promise<HeadlessChromiumDriverFactory> {
return new HeadlessChromiumDriverFactory(binaryPath, logger, captureConfig);
}
export const chromium: BrowserDownload = {
paths,
createDriverFactory: (binaryPath: string, captureConfig: CaptureConfig, logger: LevelLogger) =>
new HeadlessChromiumDriverFactory(binaryPath, captureConfig, logger),
};

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ async function ensureDownloaded(browsers: BrowserDownload[], logger: LevelLogger
const path = resolvePath(archivesPath, archiveFilename);

if (existsSync(path) && (await md5(path)) === archiveChecksum) {
logger.info(`Browser archive exists in ${path}`);
logger.debug(`Browser archive exists in ${path}`);
return;
}

Expand Down
31 changes: 24 additions & 7 deletions x-pack/plugins/reporting/server/browsers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,27 @@
* you may not use this file except in compliance with the Elastic License.
*/

import * as chromiumDefinition from './chromium';
import { first } from 'rxjs/operators';
import { LevelLogger } from '../lib';
import { CaptureConfig } from '../types';
import { chromium } from './chromium';
import { HeadlessChromiumDriverFactory } from './chromium/driver_factory';
import { installBrowser } from './install';
import { ReportingConfig } from '..';

export { ensureAllBrowsersDownloaded } from './download';
export { createBrowserDriverFactory } from './create_browser_driver_factory';

export { HeadlessChromiumDriver } from './chromium/driver';
export { HeadlessChromiumDriverFactory } from './chromium/driver_factory';
export { chromium } from './chromium';

export const chromium = {
paths: chromiumDefinition.paths,
createDriverFactory: chromiumDefinition.createDriverFactory,
};
type CreateDriverFactory = (
binaryPath: string,
captureConfig: CaptureConfig,
logger: LevelLogger
) => HeadlessChromiumDriverFactory;

export interface BrowserDownload {
createDriverFactory: CreateDriverFactory;
Copy link
Member Author

@tsullivan tsullivan Jun 4, 2020

Choose a reason for hiding this comment

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

Previously, the BrowserDownload interface wasn't being instantiated anywhere. Once I instantiated a variable as this interface, adding this createDriverFactory field was necessary.

paths: {
archivesPath: string;
baseUrl: string;
Expand All @@ -30,3 +37,13 @@ export interface BrowserDownload {
}>;
};
}

export const initializeBrowserDriverFactory = async (
config: ReportingConfig,
logger: LevelLogger
) => {
const { binaryPath$ } = installBrowser(chromium, config, logger);
const binaryPath = await binaryPath$.pipe(first()).toPromise();
const captureConfig = config.get('capture');
return chromium.createDriverFactory(binaryPath, captureConfig, logger);
};
69 changes: 48 additions & 21 deletions x-pack/plugins/reporting/server/browsers/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@

import fs from 'fs';
import path from 'path';
import * as Rx from 'rxjs';
import { first } from 'rxjs/operators';
import { promisify } from 'util';
import { ReportingConfig } from '../';
import { LevelLogger } from '../lib';
import { BrowserDownload } from './';
import { ensureBrowserDownloaded } from './download';
// @ts-ignore
import { md5 } from './download/checksum';
// @ts-ignore
Expand All @@ -19,37 +23,60 @@ const chmod = promisify(fs.chmod);
interface Package {
platforms: string[];
}
interface PathResponse {
binaryPath: string;
}

/**
* "install" a browser by type into installs path by extracting the downloaded
* archive. If there is an error extracting the archive an `ExtractError` is thrown
*/
export async function installBrowser(
logger: LevelLogger,
export function installBrowser(
browser: BrowserDownload,
installsPath: string
): Promise<PathResponse> {
const pkg = browser.paths.packages.find((p: Package) => p.platforms.includes(process.platform));
config: ReportingConfig,
logger: LevelLogger
): { binaryPath$: Rx.Subject<string> } {
const binaryPath$ = new Rx.Subject<string>();
const backgroundInstall = async () => {
const captureConfig = config.get('capture');
const { autoDownload, type: browserType } = captureConfig.browser;
if (autoDownload) {
await ensureBrowserDownloaded(browserType, logger);
}

const pkg = browser.paths.packages.find((p: Package) => p.platforms.includes(process.platform));
if (!pkg) {
throw new Error(`Unsupported platform: ${JSON.stringify(browser, null, 2)}`);
}

const dataDir = await config.kbnConfig.get('path', 'data').pipe(first()).toPromise();
const binaryPath = path.join(dataDir, pkg.binaryRelativePath);

try {
const binaryChecksum = await md5(binaryPath).catch(() => '');

if (!pkg) {
throw new Error(`Unsupported platform: ${JSON.stringify(browser, null, 2)}`);
}
if (binaryChecksum !== pkg.binaryChecksum) {
const archive = path.join(browser.paths.archivesPath, pkg.archiveFilename);
logger.info(`Extracting [${archive}] to [${binaryPath}]`);
await extract(archive, dataDir);
await chmod(binaryPath, '755');
}
} catch (error) {
if (error.cause && ['EACCES', 'EEXIST'].includes(error.cause.code)) {
logger.error(
`Error code ${error.cause.code}: Insufficient permissions for extracting the browser archive. ` +
`Make sure the Kibana data directory (path.data) is owned by the same user that is running Kibana.`
);
}

const binaryPath = path.join(installsPath, pkg.binaryRelativePath);
const binaryChecksum = await md5(binaryPath).catch(() => '');
throw error; // reject the promise with the original error
joelgriffith marked this conversation as resolved.
Show resolved Hide resolved
}

logger.debug(`Browser executable: ${binaryPath}`);

binaryPath$.next(binaryPath); // subscribers wait for download and extract to complete
};

if (binaryChecksum !== pkg.binaryChecksum) {
const archive = path.join(browser.paths.archivesPath, pkg.archiveFilename);
logger.debug(`Extracting [${archive}] to [${binaryPath}]`);
await extract(archive, installsPath);
await chmod(binaryPath, '755');
}
backgroundInstall();

logger.debug(`Browser installed at ${binaryPath}`);
return {
binaryPath,
binaryPath$,
};
}
15 changes: 10 additions & 5 deletions x-pack/plugins/reporting/server/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { Observable } from 'rxjs';
import { get } from 'lodash';
import { map } from 'rxjs/operators';
import { Observable } from 'rxjs';
import { first, map } from 'rxjs/operators';
import { CoreSetup, PluginInitializerContext } from 'src/core/server';
import { LevelLogger } from '../lib';
import { createConfig$ } from './create_config';
import { ReportingConfigType } from './schema';

// make config.get() aware of the value type it returns
Expand Down Expand Up @@ -55,11 +57,12 @@ export interface ReportingConfig extends Config<ReportingConfigType> {
kbnConfig: Config<KbnServerConfigType>;
}

export const buildConfig = (
export const buildConfig = async (
initContext: PluginInitializerContext<ReportingConfigType>,
core: CoreSetup,
reportingConfig: ReportingConfigType
): ReportingConfig => {
logger: LevelLogger
): Promise<ReportingConfig> => {
const config$ = initContext.config.create<ReportingConfigType>();
const { http } = core;
const serverInfo = http.getServerInfo();

Expand All @@ -77,6 +80,8 @@ export const buildConfig = (
},
};

const reportingConfig$ = createConfig$(core, config$, logger);
const reportingConfig = await reportingConfig$.pipe(first()).toPromise();
return {
get: (...keys: string[]) => get(reportingConfig, keys.join('.'), null), // spreading arguments as an array allows the return type to be known by the compiler
kbnConfig: {
Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/reporting/server/config/create_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ describe('Reporting server createConfig$', () => {
mockInitContext = makeMockInitContext({
kibanaServer: {},
});
mockLogger = ({ warn: jest.fn(), debug: jest.fn() } as unknown) as LevelLogger;
mockLogger = ({
warn: jest.fn(),
debug: jest.fn(),
clone: jest.fn().mockImplementation(() => mockLogger),
} as unknown) as LevelLogger;
});

afterEach(() => {
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/reporting/server/config/create_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ import { ReportingConfigType } from './schema';
export function createConfig$(
core: CoreSetup,
config$: Observable<ReportingConfigType>,
logger: LevelLogger
parentLogger: LevelLogger
) {
const logger = parentLogger.clone(['config']);
return config$.pipe(
map((config) => {
// encryption key
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/reporting/server/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import { PluginConfigDescriptor } from 'kibana/server';
import { ConfigSchema, ReportingConfigType } from './schema';
export { buildConfig } from './config';
export { createConfig$ } from './create_config';
export { ConfigSchema, ReportingConfigType };

export const config: PluginConfigDescriptor<ReportingConfigType> = {
Expand Down
11 changes: 9 additions & 2 deletions x-pack/plugins/reporting/server/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import { ESQueueInstance } from './lib/create_queue';
import { EnqueueJobFn } from './lib/enqueue_job';

export interface ReportingInternalSetup {
browserDriverFactory: HeadlessChromiumDriverFactory;
elasticsearch: ElasticsearchServiceSetup;
licensing: LicensingPluginSetup;
basePath: BasePath['get'];
Expand All @@ -44,6 +43,7 @@ interface ReportingInternalStart {
export class ReportingCore {
private pluginSetupDeps?: ReportingInternalSetup;
private pluginStartDeps?: ReportingInternalStart;
private browserDriverFactory?: HeadlessChromiumDriverFactory;
private readonly pluginSetup$ = new Rx.ReplaySubject<ReportingInternalSetup>();
private readonly pluginStart$ = new Rx.ReplaySubject<ReportingInternalStart>();
private exportTypesRegistry = getExportTypesRegistry();
Expand All @@ -63,6 +63,10 @@ export class ReportingCore {
return this.pluginStart$.pipe(first(), mapTo(true)).toPromise();
}

public setBrowserDriverFactory(browserDriverFactory: HeadlessChromiumDriverFactory) {
this.browserDriverFactory = browserDriverFactory;
}

/*
* Internal module dependencies
*/
Expand Down Expand Up @@ -93,7 +97,10 @@ export class ReportingCore {
}

public getScreenshotsObservable(): ScreenshotsObservableFn {
const { browserDriverFactory } = this.getPluginSetupDeps();
const { browserDriverFactory } = this;
if (!browserDriverFactory) {
throw new Error(`"browserDriverFactory" dependency hasn't initialized yet`);
}
return screenshotsObservableFactory(this.config.get('capture'), browserDriverFactory);
}

Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/reporting/server/lib/validate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@
import { i18n } from '@kbn/i18n';
import { ElasticsearchServiceSetup } from 'kibana/server';
import { ReportingConfig } from '../../';
import { LevelLogger } from '../../lib';
import { HeadlessChromiumDriverFactory } from '../../browsers/chromium/driver_factory';
import { LevelLogger } from '../../lib';
import { validateBrowser } from './validate_browser';
import { validateMaxContentLength } from './validate_max_content_length';

export async function runValidations(
config: ReportingConfig,
elasticsearch: ElasticsearchServiceSetup,
browserFactory: HeadlessChromiumDriverFactory,
logger: LevelLogger
parentLogger: LevelLogger
) {
const logger = parentLogger.clone(['validations']);
try {
await Promise.all([
validateBrowser(browserFactory, logger),
Expand Down
Loading