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] Server Routing and getDocumentPayload Typescript Improvements #55608

Closed
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { cryptoFactory, LevelLogger } from '../../../server/lib';
import {
ExecuteJobFactory,
ImmediateExecuteFn,
JobDocOutputExecuted,
JobDocOutput,
ServerFacade,
RequestFacade,
} from '../../../types';
Expand All @@ -36,7 +36,7 @@ export const executeJobFactory: ExecuteJobFactory<ImmediateExecuteFn<
jobId: string | null,
job: JobDocPayloadPanelCsv,
realRequest?: RequestFacade
): Promise<JobDocOutputExecuted> {
): Promise<JobDocOutput> {
// There will not be a jobID for "immediate" generation.
// jobID is only for "queued" jobs
// Use the jobID as a logging tag or "immediate"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
HeadlessChromiumDriverFactory,
ReportingResponseToolkit,
Logger,
JobDocOutputExecuted,
JobDocOutput,
} from '../../types';
import { JobDocPayloadPanelCsv } from '../../export_types/csv_from_savedobject/types';
import { getJobParamsFromRequest } from '../../export_types/csv_from_savedobject/server/lib/get_job_params_from_request';
Expand Down Expand Up @@ -68,7 +68,7 @@ export function registerGenerateCsvFromSavedObjectImmediate(
content_type: jobOutputContentType,
content: jobOutputContent,
size: jobOutputSize,
}: JobDocOutputExecuted = await executeJobFn(null, jobDocPayload, request);
}: JobDocOutput = await executeJobFn(null, jobDocPayload, request);

logger.info(`Job output size: ${jobOutputSize} bytes`);

Expand Down
42 changes: 24 additions & 18 deletions x-pack/legacy/plugins/reporting/server/routes/jobs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
* you may not use this file except in compliance with the Elastic License.
*/

import Boom from 'boom';
import { Legacy } from 'kibana';
import boom from 'boom';
import { ResponseObject } from 'hapi';
import { API_BASE_URL } from '../../common/constants';
import {
ServerFacade,
Expand All @@ -28,6 +29,10 @@ import { makeRequestFacade } from './lib/make_request_facade';

const MAIN_ENTRY = `${API_BASE_URL}/jobs`;

function isResponse(response: Boom<null> | ResponseObject): response is ResponseObject {
return !(response as Boom<unknown>).isBoom;
}

export function registerJobInfoRoutes(
server: ServerFacade,
exportTypesRegistry: ExportTypesRegistry,
Expand Down Expand Up @@ -84,14 +89,14 @@ export function registerJobInfoRoutes(
return jobsQuery.get(request.pre.user, docId, { includeContent: true }).then(
(result): JobDocOutput => {
if (!result) {
throw boom.notFound();
throw Boom.notFound();
}
const {
_source: { jobtype: jobType, output: jobOutput },
} = result;

if (!request.pre.management.jobTypes.includes(jobType)) {
throw boom.unauthorized(`Sorry, you are not authorized to download ${jobType} reports`);
throw Boom.unauthorized(`Sorry, you are not authorized to download ${jobType} reports`);
}

return jobOutput;
Expand All @@ -111,13 +116,13 @@ export function registerJobInfoRoutes(

return jobsQuery.get(request.pre.user, docId).then((result): JobSource<any>['_source'] => {
if (!result) {
throw boom.notFound();
throw Boom.notFound();
}

const { _source: job } = result;
const { jobtype: jobType, payload: jobPayload } = job;
if (!request.pre.management.jobTypes.includes(jobType)) {
throw boom.unauthorized(`Sorry, you are not authorized to view ${jobType} info`);
throw Boom.unauthorized(`Sorry, you are not authorized to view ${jobType} info`);
}

return {
Expand Down Expand Up @@ -147,21 +152,22 @@ export function registerJobInfoRoutes(
h,
{ docId }
);
const { statusCode } = response;

if (statusCode !== 200) {
if (statusCode === 500) {
logger.error(`Report ${docId} has failed: ${JSON.stringify(response.source)}`);
} else {
logger.debug(
`Report ${docId} has non-OK status: [${statusCode}] Reason: [${JSON.stringify(
response.source
)}]`
);

if (isResponse(response)) {
const { statusCode } = response;

if (statusCode !== 200) {
if (statusCode === 500) {
logger.error(`Report ${docId} has failed: ${JSON.stringify(response.source)}`);
} else {
logger.debug(
`Report ${docId} has non-OK status: [${statusCode}] Reason: [${JSON.stringify(
response.source
)}]`
);
}
}
}

if (!response.isBoom) {
response = response.header('accept-ranges', 'none');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,22 @@
*/

import expect from '@kbn/expect';
import sinon from 'sinon';
import { authorizedUserPreRoutingFactory } from '../authorized_user_pre_routing';
import { authorizedUserPreRoutingFactory } from './authorized_user_pre_routing';

describe('authorized_user_pre_routing', function() {
// the getClientShield is using `once` which forces us to use a constant mock
// which makes testing anything that is dependent on `oncePerServer` confusing.
// so createMockServer reuses the same 'instance' of the server and overwrites
// the properties to contain different values
const createMockServer = (function() {
const getUserStub = sinon.stub();
const getUserStub = jest.fn();
let mockConfig;

const mockServer = {
expose: function() {},
config: function() {
expose() {},
config() {
return {
get: function(key) {
get(key) {
return mockConfig[key];
},
};
Expand All @@ -45,7 +44,7 @@ describe('authorized_user_pre_routing', function() {
mockServer.plugins.xpack_main = {
info: !xpackInfoUndefined && {
isAvailable: () => xpackInfoAvailable,
feature: function(featureName) {
feature(featureName) {
if (featureName === 'security') {
return {
isEnabled: () => securityEnabled,
Expand All @@ -56,8 +55,8 @@ describe('authorized_user_pre_routing', function() {
},
};

getUserStub.resetHistory();
getUserStub.resolves(user);
getUserStub.mockReset();
getUserStub.mockResolvedValue(user);
return mockServer;
};
})();
Expand All @@ -66,7 +65,7 @@ describe('authorized_user_pre_routing', function() {
const mockServer = createMockServer({ xpackInfoUndefined: true });

const authorizedUserPreRouting = authorizedUserPreRoutingFactory(mockServer);
const response = await authorizedUserPreRouting();
const response = await authorizedUserPreRouting({});
expect(response.isBoom).to.be(true);
expect(response.output.statusCode).to.be(404);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,30 @@ import {
ServerFacade,
ExportTypesRegistry,
ExportTypeDefinition,
JobDocExecuted,
JobDocOutputExecuted,
JobDocOutput,
JobSource,
} from '../../../types';
import { CSV_JOB_TYPE } from '../../../common/constants';

interface ICustomHeaders {
[x: string]: any;
}

const DEFAULT_TITLE = 'report';
type ExportTypeType = ExportTypeDefinition<unknown, unknown, unknown, unknown>;

interface Payload {
statusCode: number;
content: any;
contentType: string;
headers: Record<string, any>;
}

type ExportTypeType = ExportTypeDefinition<any, any, any, any>;
const DEFAULT_TITLE = 'report';

const getTitle = (exportType: ExportTypeType, title?: string): string =>
`${title || DEFAULT_TITLE}.${exportType.jobContentExtension}`;

const getReportingHeaders = (output: JobDocOutputExecuted, exportType: ExportTypeType) => {
const getReportingHeaders = (output: JobDocOutput, exportType: ExportTypeType) => {
const metaDataHeaders: ICustomHeaders = {};

if (exportType.jobType === CSV_JOB_TYPE) {
Expand All @@ -54,7 +61,7 @@ export function getDocumentPayloadFactory(
}
}

function getCompleted(output: JobDocOutputExecuted, jobType: string, title: string) {
function getCompleted(output: JobDocOutput, jobType: string, title: string) {
const exportType = exportTypesRegistry.get((item: ExportTypeType) => item.jobType === jobType);
const filename = getTitle(exportType, title);
const headers = getReportingHeaders(output, exportType);
Expand All @@ -70,14 +77,15 @@ export function getDocumentPayloadFactory(
};
}

function getFailure(output: JobDocOutputExecuted) {
function getFailure(output: JobDocOutput) {
return {
statusCode: 500,
content: {
message: 'Reporting generation failed',
reason: output.content,
},
contentType: 'application/json',
headers: {},
};
}

Expand All @@ -90,9 +98,7 @@ export function getDocumentPayloadFactory(
};
}

return function getDocumentPayload(doc: {
_source: JobDocExecuted<{ output: JobDocOutputExecuted }>;
}) {
return function getDocumentPayload(doc: JobSource<unknown>): Payload {
const { status, jobtype: jobType, payload: { title } = { title: '' } } = doc._source;
const { output } = doc._source;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,48 @@
* you may not use this file except in compliance with the Elastic License.
*/

import boom from 'boom';
import Boom from 'boom';
import { ResponseToolkit } from 'hapi';
import { ServerFacade, ExportTypesRegistry } from '../../../types';
import { jobsQueryFactory } from '../../lib/jobs_query';
import { WHITELISTED_JOB_CONTENT_TYPES } from '../../../common/constants';
import { getDocumentPayloadFactory } from './get_document_payload';

export function jobResponseHandlerFactory(server, exportTypesRegistry) {
interface JobResponseHandlerParams {
docId: string;
}

interface JobResponseHandlerOpts {
excludeContent?: boolean;
}

export function jobResponseHandlerFactory(
server: ServerFacade,
exportTypesRegistry: ExportTypesRegistry
) {
const jobsQuery = jobsQueryFactory(server);
const getDocumentPayload = getDocumentPayloadFactory(server, exportTypesRegistry);

return function jobResponseHandler(validJobTypes, user, h, params, opts = {}) {
return function jobResponseHandler(
validJobTypes: string[],
user: any,
h: ResponseToolkit,
params: JobResponseHandlerParams,
opts: JobResponseHandlerOpts = {}
) {
const { docId } = params;
return jobsQuery.get(user, docId, { includeContent: !opts.excludeContent }).then(doc => {
if (!doc) return boom.notFound();
if (!doc) return Boom.notFound();

const { jobtype: jobType } = doc._source;
if (!validJobTypes.includes(jobType)) {
return boom.unauthorized(`Sorry, you are not authorized to download ${jobType} reports`);
return Boom.unauthorized(`Sorry, you are not authorized to download ${jobType} reports`);
}

const output = getDocumentPayload(doc);

if (!WHITELISTED_JOB_CONTENT_TYPES.includes(output.contentType)) {
return boom.badImplementation(
return Boom.badImplementation(
`Unsupported content-type of ${output.contentType} specified by job output`
);
}
Expand All @@ -42,7 +61,7 @@ export function jobResponseHandlerFactory(server, exportTypesRegistry) {
});
}

return response;
return response; // Hapi
});
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const reportingFeaturePreRoutingFactory = function reportingFeaturePreRou
return function reportingFeaturePreRouting(getReportingFeatureId: GetReportingFeatureIdFn) {
return function licensePreRouting(request: Legacy.Request) {
const licenseCheckResults = xpackMainPlugin.info.feature(pluginId).getLicenseCheckResults();
const reportingFeatureId = getReportingFeatureId(request);
const reportingFeatureId = getReportingFeatureId(request) as string;
const reportingFeature = licenseCheckResults[reportingFeatureId];
if (!reportingFeature.showLinks || !reportingFeature.enableLinks) {
throw Boom.forbidden(reportingFeature.message);
Expand Down
34 changes: 5 additions & 29 deletions x-pack/legacy/plugins/reporting/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,18 +200,6 @@ export interface JobDocPayload<JobParamsType> {
type: string | null;
}

export interface JobDocOutput {
content: string; // encoded content
contentType: string;
}

export interface JobDocExecuted<JobParamsType> {
jobtype: string;
output: JobDocOutputExecuted;
payload: JobDocPayload<JobParamsType>;
status: string; // completed, failed, etc
}

export interface JobSource<JobParamsType> {
_id: string;
_source: {
Expand All @@ -222,21 +210,9 @@ export interface JobSource<JobParamsType> {
};
}

/*
* A snake_cased field is the only significant difference in structure of
* JobDocOutputExecuted vs JobDocOutput.
*
* JobDocOutput is the structure of the object returned by getDocumentPayload
*
* data in the _source fields of the
* Reporting index.
*
* The ESQueueWorker internals have executed job objects returned with this
* structure. See `_formatOutput` in reporting/server/lib/esqueue/worker.js
*/
export interface JobDocOutputExecuted {
content_type: string; // vs `contentType` above
content: string | null; // defaultOutput is null
export interface JobDocOutput {
content_type: string;
content: string | null;
max_size_reached: boolean;
size: number;
}
Expand Down Expand Up @@ -279,7 +255,7 @@ export type ImmediateExecuteFn<JobParamsType> = (
jobId: null,
job: JobDocPayload<JobParamsType>,
request: RequestFacade
) => Promise<JobDocOutputExecuted>;
) => Promise<JobDocOutput>;

export interface ESQueueWorkerOptions {
kibanaName: string;
Expand All @@ -292,7 +268,7 @@ export interface ESQueueWorkerOptions {
type GenericWorkerFn<JobParamsType> = (
jobSource: JobSource<JobParamsType>,
...workerRestArgs: any[]
) => void | Promise<JobDocOutputExecuted>;
) => void | Promise<JobDocOutput>;

export interface ESQueueInstance<JobParamsType, JobDocPayloadType> {
registerWorker: (
Expand Down