-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Endpoint][EPM] Retrieve Index Pattern from Ingest Manager #63016
Changes from 30 commits
b69c2ad
7c7eaee
c9f0507
b82762b
cb45dfe
9e4f6e3
8cbfa42
a1f2bd0
f13231a
0c9946d
0140395
6c7b7b4
745c15d
661f860
b839152
cf1a071
9ff1e7b
8272634
3599f46
6c22146
6457c0f
3fd2dc1
4d2e8ce
e94629f
89dd822
337ab45
9ced2c8
4dc5854
b669294
cc4a355
1aa9388
dd033d4
efdcc2d
2829689
6facd1f
ec0a649
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 |
---|---|---|
@@ -0,0 +1,9 @@ | ||
/* | ||
* 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 { schema } from '@kbn/config-schema'; | ||
|
||
export const indexPatternGetParamsSchema = schema.object({ datasetPath: schema.string() }); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
/* | ||
* 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 { Logger, LoggerFactory, RequestHandlerContext } from 'kibana/server'; | ||
import { ESIndexPatternService } from '../../ingest_manager/server'; | ||
import { EndpointAppConstants } from '../common/types'; | ||
|
||
export interface IndexPatternRetriever { | ||
getIndexPattern(ctx: RequestHandlerContext, datasetPath: string): Promise<string>; | ||
getEventIndexPattern(ctx: RequestHandlerContext): Promise<string>; | ||
getMetadataIndexPattern(ctx: RequestHandlerContext): Promise<string>; | ||
} | ||
|
||
export class IngestIndexPatternRetriever implements IndexPatternRetriever { | ||
jonathan-buttner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private static endpointPackageName = 'endpoint'; | ||
private static metadataDataset = 'metadata'; | ||
private readonly log: Logger; | ||
constructor(private readonly service: ESIndexPatternService, loggerFactory: LoggerFactory) { | ||
this.log = loggerFactory.get('index-pattern-retriever'); | ||
} | ||
|
||
async getEventIndexPattern(ctx: RequestHandlerContext) { | ||
return await this.getIndexPattern(ctx, EndpointAppConstants.EVENT_DATASET); | ||
} | ||
|
||
async getMetadataIndexPattern(ctx: RequestHandlerContext) { | ||
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. would you mind adding a quick doc comment to public methods? maybe
I think these types of comments will help a lot, esp. considering the size of the org |
||
return await this.getIndexPattern(ctx, IngestIndexPatternRetriever.metadataDataset); | ||
} | ||
|
||
async getIndexPattern(ctx: RequestHandlerContext, datasetPath: string) { | ||
const pattern = await this.service.getESIndexPattern( | ||
ctx.core.savedObjects.client, | ||
IngestIndexPatternRetriever.endpointPackageName, | ||
datasetPath | ||
); | ||
|
||
if (!pattern) { | ||
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. ❔ What happens if something inside 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. Ah thanks for catching that, I'll wrap it in a |
||
const msg = `Unable to retrieve the index pattern for dataset: ${datasetPath}`; | ||
this.log.warn(msg); | ||
throw new Error(msg); | ||
} | ||
|
||
return pattern; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
/* | ||
* 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. | ||
*/ | ||
|
||
export const createMockIndexPatternRetriever = (indexPattern: string) => { | ||
jonathan-buttner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const mockGetFunc = jest.fn().mockResolvedValue(indexPattern); | ||
return { | ||
getIndexPattern: mockGetFunc, | ||
getEventIndexPattern: mockGetFunc, | ||
getMetadataIndexPattern: mockGetFunc, | ||
}; | ||
}; | ||
|
||
export const MetadataIndexPattern = 'metrics-endpoint-*'; | ||
|
||
export const createMockMetadataIPRetriever = () => { | ||
jonathan-buttner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return createMockIndexPatternRetriever(MetadataIndexPattern); | ||
}; | ||
|
||
export const createMockIndexPatternService = (indexPattern: string) => { | ||
return { | ||
esIndexPatternService: { | ||
getESIndexPattern: jest.fn().mockResolvedValue(indexPattern), | ||
}, | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
/* | ||
* 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 { IRouter, Logger, RequestHandler } from 'kibana/server'; | ||
import { EndpointAppContext } from '../types'; | ||
import { IndexPatternGetParamsResult, EndpointAppConstants } from '../../common/types'; | ||
import { indexPatternGetParamsSchema } from '../../common/schema/index_pattern'; | ||
import { IndexPatternRetriever } from '../index_pattern'; | ||
|
||
function handleIndexPattern( | ||
log: Logger, | ||
indexRetriever: IndexPatternRetriever | ||
): RequestHandler<IndexPatternGetParamsResult> { | ||
return async (context, req, res) => { | ||
try { | ||
return res.ok({ | ||
body: { | ||
indexPattern: await indexRetriever.getIndexPattern(context, req.params.datasetPath), | ||
}, | ||
}); | ||
} catch (error) { | ||
log.warn(error); | ||
return res.notFound({ body: error }); | ||
} | ||
}; | ||
} | ||
|
||
export function registerIndexPatternRoute(router: IRouter, endpointAppContext: EndpointAppContext) { | ||
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. is this for the alert list feature? if so, thoughts on moving it to the alerting routes module? 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. Yeah it's currently only used for the alert list. The alert list uses an index pattern when doing time based filtering I think. I think having an index pattern is required when doing that. I could foresee other pages (Host maybe?) needing an index pattern if they want to do any time based filtering too. If you'd prefer it in the alert list for now until it's needed in the future, that's fine too though. |
||
const log = endpointAppContext.logFactory.get('index_pattern'); | ||
|
||
router.get( | ||
{ | ||
path: `${EndpointAppConstants.INDEX_PATTERN_ROUTE}/{datasetPath}`, | ||
validate: { params: indexPatternGetParamsSchema }, | ||
options: { authRequired: true }, | ||
}, | ||
handleIndexPattern(log, endpointAppContext.indexPatternRetriever) | ||
); | ||
} |
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.
🛑 @kqualters-elastic and @marshallmain changed this to an
import
, which works better I think. I think it was merged upstream?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.
ah, let me take a look
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.
no, that was not done
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.
Yeah, @kqualters-elastic is right, we're not using an
import
I'll revert this toevents-endpoint-1
though because until the alert details issue get's refactored: https://github.com/elastic/endpoint-app-team/issues/311 it has to stay asevents-endpoint-1