Skip to content

Commit

Permalink
[Reporting] Use spaceId from request in export generation (#76998) (#…
Browse files Browse the repository at this point in the history
…77952)

* [Reporting] Use spaceId from request in export generation

* remove todo that has been done

* whitespace

* use post params api in test

* add logging to core

* Update x-pack/plugins/reporting/server/export_types/printable_pdf/lib/get_custom_logo.ts

Co-authored-by: Joel Griffith <[email protected]>

* more logging

* fix interdependence and remove Promise.all

* getAbsoluteUrl have only 1 way to provide basePath

* --wip-- [skip ci]

* log apipath

* deleteAllReports at the end

* tests pass locally

* set config in the tests

* re-add skips of flaky tests

* test using csv:quoteValues

Co-authored-by: Joel Griffith <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Joel Griffith <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
3 people authored Sep 19, 2020
1 parent 765ac06 commit 97af0fc
Show file tree
Hide file tree
Showing 40 changed files with 252 additions and 378 deletions.
2 changes: 2 additions & 0 deletions x-pack/plugins/reporting/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ export const KBN_SCREENSHOT_HEADER_BLOCK_LIST = [
export const KBN_SCREENSHOT_HEADER_BLOCK_LIST_STARTS_WITH_PATTERN = ['proxy-'];

export const UI_SETTINGS_CUSTOM_PDF_LOGO = 'xpackReporting:customPdfLogo';
export const UI_SETTINGS_CSV_SEPARATOR = 'csv:separator';
export const UI_SETTINGS_CSV_QUOTE_VALUES = 'csv:quoteValues';

export const PDF_JOB_TYPE = 'printable_pdf';
export const PNG_JOB_TYPE = 'PNG';
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/reporting/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"kibanaVersion": "kibana",
"optionalPlugins": [
"security",
"spaces",
"usageCollection"
],
"configPath": ["xpack", "reporting"],
Expand Down
62 changes: 55 additions & 7 deletions x-pack/plugins/reporting/server/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import Hapi from 'hapi';
import * as Rx from 'rxjs';
import { first, map, take } from 'rxjs/operators';
import {
Expand All @@ -14,24 +15,27 @@ import {
SavedObjectsClientContract,
SavedObjectsServiceStart,
UiSettingsServiceStart,
} from 'src/core/server';
} from '../../../../src/core/server';
import { PluginSetupContract as FeaturesPluginSetup } from '../../features/server';
import { LicensingPluginSetup } from '../../licensing/server';
import { SecurityPluginSetup } from '../../security/server';
import { DEFAULT_SPACE_ID } from '../../spaces/common/constants';
import { SpacesPluginSetup } from '../../spaces/server';
import { ReportingConfig } from './';
import { HeadlessChromiumDriverFactory } from './browsers/chromium/driver_factory';
import { checkLicense, getExportTypesRegistry } from './lib';
import { checkLicense, getExportTypesRegistry, LevelLogger } from './lib';
import { ESQueueInstance } from './lib/create_queue';
import { screenshotsObservableFactory, ScreenshotsObservableFn } from './lib/screenshots';
import { ReportingStore } from './lib/store';

export interface ReportingInternalSetup {
basePath: Pick<BasePath, 'set'>;
router: IRouter;
features: FeaturesPluginSetup;
elasticsearch: ElasticsearchServiceSetup;
licensing: LicensingPluginSetup;
basePath: BasePath['get'];
router: IRouter;
security?: SecurityPluginSetup;
spaces?: SpacesPluginSetup;
}

export interface ReportingInternalStart {
Expand All @@ -50,7 +54,7 @@ export class ReportingCore {
private exportTypesRegistry = getExportTypesRegistry();
private config?: ReportingConfig;

constructor() {}
constructor(private logger: LevelLogger) {}

/*
* Register setupDeps
Expand Down Expand Up @@ -180,14 +184,58 @@ export class ReportingCore {
return this.getPluginSetupDeps().elasticsearch;
}

public async getSavedObjectsClient(fakeRequest: KibanaRequest) {
private async getSavedObjectsClient(request: KibanaRequest) {
const { savedObjects } = await this.getPluginStartDeps();
return savedObjects.getScopedClient(fakeRequest) as SavedObjectsClientContract;
return savedObjects.getScopedClient(request) as SavedObjectsClientContract;
}

public async getUiSettingsServiceFactory(savedObjectsClient: SavedObjectsClientContract) {
const { uiSettings: uiSettingsService } = await this.getPluginStartDeps();
const scopedUiSettingsService = uiSettingsService.asScopedToClient(savedObjectsClient);
return scopedUiSettingsService;
}

public getSpaceId(request: KibanaRequest): string | undefined {
const spacesService = this.getPluginSetupDeps().spaces?.spacesService;
if (spacesService) {
const spaceId = spacesService?.getSpaceId(request);

if (spaceId !== DEFAULT_SPACE_ID) {
this.logger.info(`Request uses Space ID: ` + spaceId);
return spaceId;
} else {
this.logger.info(`Request uses default Space`);
}
}
}

public getFakeRequest(baseRequest: object, spaceId?: string) {
const fakeRequest = KibanaRequest.from({
path: '/',
route: { settings: {} },
url: { href: '/' },
raw: { req: { url: '/' } },
...baseRequest,
} as Hapi.Request);

const spacesService = this.getPluginSetupDeps().spaces?.spacesService;
if (spacesService) {
if (spaceId && spaceId !== DEFAULT_SPACE_ID) {
this.logger.info(`Generating request for space: ` + spaceId);
this.getPluginSetupDeps().basePath.set(fakeRequest, `/s/${spaceId}`);
}
}

return fakeRequest;
}

public async getUiSettingsClient(request: KibanaRequest) {
const spacesService = this.getPluginSetupDeps().spaces?.spacesService;
const spaceId = this.getSpaceId(request);
if (spacesService && spaceId) {
this.logger.info(`Creating UI Settings Client for space: ${spaceId}`);
}
const savedObjectsClient = await this.getSavedObjectsClient(request);
return await this.getUiSettingsServiceFactory(savedObjectsClient);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ interface HasEncryptedHeaders {
headers?: string;
}

// TODO merge functionality with CSV execute job
export const decryptJobHeaders = async <
JobParamsType,
TaskPayloadType extends HasEncryptedHeaders
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { getAbsoluteUrlFactory } from './get_absolute_url';

const defaultOptions = {
defaultBasePath: 'sbp',
basePath: 'sbp',
protocol: 'http:',
hostname: 'localhost',
port: 5601,
Expand Down Expand Up @@ -64,8 +64,8 @@ test(`uses the provided hash with queryString`, () => {
});

test(`uses the provided basePath`, () => {
const getAbsoluteUrl = getAbsoluteUrlFactory(defaultOptions);
const absoluteUrl = getAbsoluteUrl({ basePath: '/s/marketing' });
const getAbsoluteUrl = getAbsoluteUrlFactory({ ...defaultOptions, basePath: '/s/marketing' });
const absoluteUrl = getAbsoluteUrl();
expect(absoluteUrl).toBe(`http://localhost:5601/s/marketing/app/kibana`);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import url from 'url';

interface AbsoluteURLFactoryOptions {
defaultBasePath: string;
basePath: string;
protocol: string;
hostname: string;
port: string | number;
Expand All @@ -17,14 +17,9 @@ export const getAbsoluteUrlFactory = ({
protocol,
hostname,
port,
defaultBasePath,
basePath,
}: AbsoluteURLFactoryOptions) => {
return function getAbsoluteUrl({
basePath = defaultBasePath,
hash = '',
path = '/app/kibana',
search = '',
} = {}) {
return function getAbsoluteUrl({ hash = '', path = '/app/kibana', search = '' } = {}) {
return url.format({
protocol,
hostname,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,16 @@
*/

import { ReportingConfig } from '../../';
import { ReportingCore } from '../../core';
import {
createMockConfig,
createMockConfigSchema,
createMockReportingCore,
} from '../../test_helpers';
import { createMockConfig, createMockConfigSchema } from '../../test_helpers';
import { BasePayload } from '../../types';
import { TaskPayloadPDF } from '../printable_pdf/types';
import { getConditionalHeaders, getCustomLogo } from './';
import { getConditionalHeaders } from './';

let mockConfig: ReportingConfig;
let mockReportingPlugin: ReportingCore;

beforeEach(async () => {
const reportingConfig = { kibanaServer: { hostname: 'custom-hostname' } };
const mockSchema = createMockConfigSchema(reportingConfig);
mockConfig = createMockConfig(mockSchema);
mockReportingPlugin = await createMockReportingCore(mockConfig);
});

describe('conditions', () => {
Expand All @@ -32,7 +24,7 @@ describe('conditions', () => {
baz: 'quix',
};

const conditionalHeaders = await getConditionalHeaders({
const conditionalHeaders = getConditionalHeaders({
job: {} as BasePayload<any>,
filteredHeaders: permittedHeaders,
config: mockConfig,
Expand All @@ -51,83 +43,6 @@ describe('conditions', () => {
});
});

test('uses basePath from job when creating saved object service', async () => {
const mockGetSavedObjectsClient = jest.fn();
mockReportingPlugin.getSavedObjectsClient = mockGetSavedObjectsClient;

const permittedHeaders = {
foo: 'bar',
baz: 'quix',
};
const conditionalHeaders = await getConditionalHeaders({
job: {} as BasePayload<any>,
filteredHeaders: permittedHeaders,
config: mockConfig,
});
const jobBasePath = '/sbp/s/marketing';
await getCustomLogo({
reporting: mockReportingPlugin,
job: { basePath: jobBasePath } as TaskPayloadPDF,
conditionalHeaders,
config: mockConfig,
});

const getBasePath = mockGetSavedObjectsClient.mock.calls[0][0].getBasePath;
expect(getBasePath()).toBe(jobBasePath);
});

test(`uses basePath from server if job doesn't have a basePath when creating saved object service`, async () => {
const mockGetSavedObjectsClient = jest.fn();
mockReportingPlugin.getSavedObjectsClient = mockGetSavedObjectsClient;

const reportingConfig = { kibanaServer: { hostname: 'localhost' }, server: { basePath: '/sbp' } };
const mockSchema = createMockConfigSchema(reportingConfig);
mockConfig = createMockConfig(mockSchema);

const permittedHeaders = {
foo: 'bar',
baz: 'quix',
};
const conditionalHeaders = await getConditionalHeaders({
job: {} as BasePayload<any>,
filteredHeaders: permittedHeaders,
config: mockConfig,
});

await getCustomLogo({
reporting: mockReportingPlugin,
job: {} as TaskPayloadPDF,
conditionalHeaders,
config: mockConfig,
});

const getBasePath = mockGetSavedObjectsClient.mock.calls[0][0].getBasePath;
expect(getBasePath()).toBe(`/sbp`);
expect(mockGetSavedObjectsClient.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Object {
"getBasePath": [Function],
"headers": Object {
"baz": "quix",
"foo": "bar",
},
"path": "/",
"raw": Object {
"req": Object {
"url": "/",
},
},
"route": Object {
"settings": Object {},
},
"url": Object {
"href": "/",
},
},
]
`);
});

describe('config formatting', () => {
test(`lowercases kibanaServer.hostname`, async () => {
const reportingConfig = { kibanaServer: { hostname: 'GREAT-HOSTNAME' } };
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,7 @@ export function getFullUrls<TaskPayloadType>({
config.get('kibanaServer', 'hostname'),
config.get('kibanaServer', 'port'),
] as string[];
const getAbsoluteUrl = getAbsoluteUrlFactory({
defaultBasePath: basePath,
protocol,
hostname,
port,
});
const getAbsoluteUrl = getAbsoluteUrlFactory({ basePath, protocol, hostname, port });

// PDF and PNG job params put in the url differently
let relativeUrls: string[] = [];
Expand All @@ -61,7 +56,6 @@ export function getFullUrls<TaskPayloadType>({
const urls = relativeUrls.map((relativeUrl) => {
const parsedRelative: UrlWithStringQuery = urlParse(relativeUrl);
const jobUrl = getAbsoluteUrl({
basePath: job.basePath,
path: parsedRelative.pathname,
hash: parsedRelative.hash,
search: parsedRelative.search,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

export { decryptJobHeaders } from './decrypt_job_headers';
export { getConditionalHeaders } from './get_conditional_headers';
export { getCustomLogo } from './get_custom_logo';
export { getFullUrls } from './get_full_urls';
export { omitBlockedHeaders } from './omit_blocked_headers';
export { validateUrls } from './validate_urls';
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const createJobFnFactory: CreateJobFnFactory<CreateJobFn<

return {
headers: serializedEncryptedHeaders,
spaceId: reporting.getSpaceId(request),
indexPatternSavedObject,
...jobParams,
};
Expand Down
Loading

0 comments on commit 97af0fc

Please sign in to comment.