[EPM] handle unremovable packages #64096
Conversation
neptunian
added
v8.0.0
v7.8.0
release_note:skip
| const installedObjects = installation?.installed || []; | ||
| if (!installation) throw new Error('integration does not exist'); | ||
| if (installation.removable === false) | ||
| throw new Error(`The ${pkgName} integration is installed by default and cannot be removed`); |
There was a problem hiding this comment.
Could we throw BadRequest error so we have a 400 it's a user error?
There was a problem hiding this comment.
If someone tries to do this through the API, what response code does he get currently?
There was a problem hiding this comment.
@jfsiii Any thoughts on which error code we should return?
Overall, I would not block the PR on which error code we should return. Which API error codes we return where we should probably have a general look.
There was a problem hiding this comment.
@nchaulet Because NP http interface does not have native support for converting Boom exceptions into HTTP responses I am thinking we need some kind of helper for this. Otherwise we are having to check for each type of error? https://github.com/elastic/kibana/blob/master/x-pack/plugins/ingest_manager/server/routes/agent/handlers.ts#L57 . Perhaps we can always through Boom errors and always return custom errors that pass along Boom results?
There was a problem hiding this comment.
I think a 4xx is appropriate because it's a client error vs a server error. At no time will the request succeed, so we should make sure the client knows the issue is on their end.
Any of 400, 403, or 405 are good, IMO.
How about 400 for now and I/we can come back to it during the API consistency ticket for Beta?
There was a problem hiding this comment.
Yes it need some custom code in the handler I did it like this for some fleet APIs https://github.com/elastic/kibana/blob/master/x-pack/plugins/ingest_manager/server/routes/agent/handlers.ts#L122
💚 Build SucceededTo update your PR or re-run it, just comment with: |
* remove endpoint handles unremovable packages * adjust UI to disallow removing of unremovable packages
| const installedObjects = installation?.installed || []; | ||
| if (!installation) throw new Error('integration does not exist'); | ||
| if (installation.removable === false) | ||
| throw new Error(`The ${pkgName} integration is installed by default and cannot be removed`); |
There was a problem hiding this comment.
I think a 4xx is appropriate because it's a client error vs a server error. At no time will the request succeed, so we should make sure the client knows the issue is on their end.
Any of 400, 403, or 405 are good, IMO.
How about 400 for now and I/we can come back to it during the API consistency ticket for Beta?
#63530
DELETE http://localhost:5603/ssg/api/ingest_manager/epm/packages/system-0.9.0
Whether or not datasources are connected to an unremovable package:
