-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Remove any types and references to Hapi #49250
Changes from 1 commit
470c47f
ba10077
b0ad892
b1a9b04
4965b66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,15 +5,19 @@ | |
*/ | ||
|
||
import crypto from 'crypto'; | ||
import { Logger } from '../../../types'; | ||
import { ServerFacade, Logger } from '../../../types'; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a tough one since it's setting and defaulting a config... I wonder if we can push this in our joi schema somehow? EDIT: This was said with the thought that we should insulate ourselves from the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, it is tough because we could do more here to protect the environment from bad things happening. But giving them a made-up encryptionKey on the fly does help the first-time user experience. |
||
export function validateConfig(serverFacade: ServerFacade, logger: Logger) { | ||
const config = serverFacade.config(); | ||
|
||
export function validateConfig(config: any, logger: Logger) { | ||
const encryptionKey = config.get('xpack.reporting.encryptionKey'); | ||
if (encryptionKey == null) { | ||
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` | ||
); | ||
config.set('xpack.reporting.encryptionKey', crypto.randomBytes(16).toString('hex')); | ||
|
||
// @ts-ignore: No set() method on KibanaConfig, just get() and has() | ||
config.set('xpack.reporting.encryptionKey', crypto.randomBytes(16).toString('hex')); // update config in memory to contain a usable encryption key | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, this part in particular might need a followup, if |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,12 @@ | |
|
||
// @ts-ignore | ||
import contentDisposition from 'content-disposition'; | ||
import * as _ from 'lodash'; | ||
import { | ||
ServerFacade, | ||
ExportTypeDefinition, | ||
JobDocExecuted, | ||
JobDocOutputExecuted, | ||
} from '../../../types'; | ||
import { oncePerServer } from '../../lib/once_per_server'; | ||
import { CSV_JOB_TYPE } from '../../../common/constants'; | ||
|
||
|
@@ -16,10 +21,10 @@ interface ICustomHeaders { | |
|
||
const DEFAULT_TITLE = 'report'; | ||
|
||
const getTitle = (exportType: any, title?: string): string => | ||
const getTitle = (exportType: ExportTypeDefinition, title?: string): string => | ||
`${title || DEFAULT_TITLE}.${exportType.jobContentExtension}`; | ||
|
||
const getReportingHeaders = (output: any, exportType: any) => { | ||
const getReportingHeaders = (output: JobDocOutputExecuted, exportType: ExportTypeDefinition) => { | ||
const metaDataHeaders: ICustomHeaders = {}; | ||
|
||
if (exportType.jobType === CSV_JOB_TYPE) { | ||
|
@@ -33,20 +38,22 @@ const getReportingHeaders = (output: any, exportType: any) => { | |
return metaDataHeaders; | ||
}; | ||
|
||
function getDocumentPayloadFn(server: any) { | ||
const exportTypesRegistry = server.plugins.reporting.exportTypesRegistry; | ||
function getDocumentPayloadFn(server: ServerFacade) { | ||
const exportTypesRegistry = server.plugins.reporting!.exportTypesRegistry; | ||
|
||
function encodeContent(content: string, exportType: any) { | ||
function encodeContent(content: string | null, exportType: ExportTypeDefinition) { | ||
switch (exportType.jobContentEncoding) { | ||
case 'base64': | ||
return Buffer.from(content, 'base64'); | ||
return content ? Buffer.from(content, 'base64') : content; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an interesting change, could you explain why we only do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is because |
||
default: | ||
return content; | ||
} | ||
} | ||
|
||
function getCompleted(output: any, jobType: string, title: any) { | ||
const exportType = exportTypesRegistry.get((item: any) => item.jobType === jobType); | ||
function getCompleted(output: JobDocOutputExecuted, jobType: string, title: string) { | ||
const exportType = exportTypesRegistry.get( | ||
(item: ExportTypeDefinition) => item.jobType === jobType | ||
); | ||
const filename = getTitle(exportType, title); | ||
const headers = getReportingHeaders(output, exportType); | ||
|
||
|
@@ -61,7 +68,7 @@ function getDocumentPayloadFn(server: any) { | |
}; | ||
} | ||
|
||
function getFailure(output: any) { | ||
function getFailure(output: JobDocOutputExecuted) { | ||
return { | ||
statusCode: 500, | ||
content: { | ||
|
@@ -72,19 +79,18 @@ function getDocumentPayloadFn(server: any) { | |
}; | ||
} | ||
|
||
function getIncomplete(status: any) { | ||
function getIncomplete(status: string) { | ||
return { | ||
statusCode: 503, | ||
content: status, | ||
contentType: 'application/json', | ||
headers: { | ||
'retry-after': 30, | ||
}, | ||
headers: { 'retry-after': 30 }, | ||
}; | ||
} | ||
|
||
return function getDocumentPayload(doc: any) { | ||
const { status, output, jobtype: jobType, payload: { title } = { title: '' } } = doc._source; | ||
return function getDocumentPayload(doc: { _source: JobDocExecuted }) { | ||
const { status, jobtype: jobType, payload: { title } = { title: '' } } = doc._source; | ||
const { output } = doc._source as { output: JobDocOutputExecuted }; | ||
|
||
if (status === 'completed') { | ||
return getCompleted(output, jobType, title); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe:
arg1
=>job
arg2
=>cancelToken
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a great part of the code - without Typescript, some bad design problems were hidden.
The idea is "worker functions" can come from different export types, but one export type changes the idea about what a worker function could be, that's the "immediate" execute type, which is in CSV export from Panel Action and doesn't go through job queuing. It registers a worker function that has a different signature. It'll be great to figure this out better when ESQueue goes away :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to leave this as-is because the arguments don't have a single semantic meaning