-
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
Server-side create/update ingest pipelines #62744
Changes from all commits
0ab60c1
c93c07b
d84b9f8
edaf515
5d65c88
664c6ad
d6f862c
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,83 @@ | ||
/* | ||
* 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 { i18n } from '@kbn/i18n'; | ||
import { schema } from '@kbn/config-schema'; | ||
|
||
import { Pipeline } from '../../../common/types'; | ||
import { API_BASE_PATH } from '../../../common/constants'; | ||
import { RouteDependencies } from '../../types'; | ||
|
||
const bodySchema = schema.object({ | ||
name: schema.string(), | ||
description: schema.string(), | ||
processors: schema.arrayOf(schema.recordOf(schema.string(), schema.any())), | ||
version: schema.maybe(schema.number()), | ||
onFailure: schema.maybe(schema.arrayOf(schema.recordOf(schema.string(), schema.any()))), | ||
}); | ||
|
||
export const registerCreateRoute = ({ | ||
router, | ||
license, | ||
lib: { isEsError }, | ||
}: RouteDependencies): void => { | ||
router.put( | ||
{ | ||
path: API_BASE_PATH, | ||
validate: { | ||
body: bodySchema, | ||
}, | ||
}, | ||
license.guardApiRoute(async (ctx, req, res) => { | ||
const { callAsCurrentUser } = ctx.core.elasticsearch.dataClient; | ||
const pipeline = req.body as Pipeline; | ||
|
||
const { name, description, processors, version, onFailure } = pipeline; | ||
|
||
try { | ||
// Check that a pipeline with the same name doesn't already exist | ||
const pipelineByName = await callAsCurrentUser('ingest.getPipeline', { id: name }); | ||
|
||
if (pipelineByName[name]) { | ||
return res.conflict({ | ||
body: new Error( | ||
i18n.translate('xpack.ingestPipelines.createRoute.duplicatePipelineIdErrorMessage', { | ||
defaultMessage: "There is already a pipeline with name '{name}'.", | ||
values: { | ||
name, | ||
}, | ||
}) | ||
), | ||
}); | ||
} | ||
} catch (e) { | ||
// Silently swallow error | ||
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. Could we check for 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 don't know if we would want to block on this. WDYT? 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'm going to leave this as is for now, so I can merge this PR and start working on the UI. I'm open to revisiting it in a follow-up PR though. |
||
} | ||
|
||
try { | ||
const response = await callAsCurrentUser('ingest.putPipeline', { | ||
id: name, | ||
body: { | ||
description, | ||
processors, | ||
version, | ||
on_failure: onFailure, | ||
}, | ||
}); | ||
|
||
return res.ok({ body: response }); | ||
} catch (error) { | ||
if (isEsError(error)) { | ||
return res.customError({ | ||
statusCode: error.statusCode, | ||
body: error, | ||
}); | ||
} | ||
|
||
return res.internalError({ body: error }); | ||
} | ||
}) | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
/* | ||
* 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'; | ||
|
||
import { Pipeline } from '../../../common/types'; | ||
import { API_BASE_PATH } from '../../../common/constants'; | ||
import { RouteDependencies } from '../../types'; | ||
|
||
const bodySchema = schema.object({ | ||
description: schema.string(), | ||
processors: schema.arrayOf(schema.recordOf(schema.string(), schema.any())), | ||
version: schema.maybe(schema.number()), | ||
onFailure: schema.maybe(schema.arrayOf(schema.recordOf(schema.string(), schema.any()))), | ||
}); | ||
|
||
const paramsSchema = schema.object({ | ||
name: schema.string(), | ||
}); | ||
|
||
export const registerUpdateRoute = ({ | ||
router, | ||
license, | ||
lib: { isEsError }, | ||
}: RouteDependencies): void => { | ||
router.put( | ||
{ | ||
path: `${API_BASE_PATH}/{name}`, | ||
validate: { | ||
body: bodySchema, | ||
params: paramsSchema, | ||
}, | ||
}, | ||
license.guardApiRoute(async (ctx, req, res) => { | ||
const { callAsCurrentUser } = ctx.core.elasticsearch.dataClient; | ||
const { name } = req.params; | ||
const pipeline = req.body as Pipeline; | ||
|
||
const { description, processors, version, onFailure } = pipeline; | ||
|
||
try { | ||
// Verify pipeline exists; ES will throw 404 if it doesn't | ||
await callAsCurrentUser('ingest.getPipeline', { id: name }); | ||
|
||
const response = await callAsCurrentUser('ingest.putPipeline', { | ||
id: name, | ||
body: { | ||
description, | ||
processors, | ||
version, | ||
on_failure: onFailure, | ||
}, | ||
}); | ||
|
||
return res.ok({ body: response }); | ||
} catch (error) { | ||
if (isEsError(error)) { | ||
return res.customError({ | ||
statusCode: error.statusCode, | ||
body: error, | ||
}); | ||
} | ||
|
||
return res.internalError({ body: error }); | ||
} | ||
}) | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
/* | ||
* 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 { FtrProviderContext } from '../../../ftr_provider_context'; | ||
|
||
export default function({ loadTestFile }: FtrProviderContext) { | ||
describe('Ingest Node Pipelines', () => { | ||
loadTestFile(require.resolve('./ingest_pipelines')); | ||
}); | ||
} |
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.
To follow REST principles it would be better to use
POST
to create a resource. Tiny change but it goes a long way 😄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.
Good point. I can change this in a follow-up PR. I think I may have used
put
here originally to align with the underlying ES API.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.
Change made via 6bba3da