Skip to content

Ship the ingest-attachment plugin by default ? #85474

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

Closed
jakelandis opened this issue Mar 29, 2022 · 6 comments
Closed

Ship the ingest-attachment plugin by default ? #85474

jakelandis opened this issue Mar 29, 2022 · 6 comments
Labels
:Core/Infra/Plugins Plugin API and infrastructure :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >enhancement Team:Core/Infra Meta label for core/infra team Team:Data Management Meta label for data/management team

Comments

@jakelandis
Copy link
Contributor

jakelandis commented Mar 29, 2022

Currently the ingest-attachment plugin requires installation via the plugin tooling. This is a fully supported, first party plugin and is already part of the Elasticsearch code base. This issue is to investigate the feasibility of converting the plugin to a module which keeps many of the characteristics of a plugin but would be available by default.

Historically, the geoip processor, user agent processor, and some repository plugins have moved from plugins to modules so there is some precedence here.

This is fairly popular plugin and there are some Elastic use-cases which could benefit from not requiring the additional install step(s).

Some immediate possible concerns:

  • non-trival increased dependency footprint - even with class loader isolation this could increase the number of possible vulnerabilities (or at least flagged by scanners)
  • increased size of assembly (tar.gz) - I measured ~34MB increase in size
  • additional security context permissions - as seen when installing the plugin
  • Others ?
@jakelandis jakelandis added >enhancement :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP labels Mar 29, 2022
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Mar 29, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@jakelandis jakelandis added the :Core/Infra/Plugins Plugin API and infrastructure label Mar 29, 2022
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Mar 29, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@jakelandis
Copy link
Contributor Author

We discussed this in person and will not include this as a default module. The additional size, scope, and usage of the plugin are not inline with general consumption.

The much higher number of dependencies would increase the security risk for all clusters (as opposed to a small subset). Additionally we have some safeguards in place w.r.t. the request sizes and heap usage, but the normal usage pattern with the ingest attachment could stress those safegaurds placing cluster stability at risk for the general case. The recommendation for dedicated ingest nodes is founded in the large consumption of compute resources and solving that problem in the general case is outside the scope of just including the plugin as a default module.

For those reasons, it seems that ingest attachment plugin is not good fit for the general default case but will remain a supported plugin.

@jakelandis
Copy link
Contributor Author

Reopening issue. After further discussion there seems to be a path forward to introduce the ingest attachment processor as a default module. Many of the concerns can be mitigated and there is significant value of having this as a default feature.

@jakelandis jakelandis reopened this Apr 26, 2022
@serenachou
Copy link
Contributor

hey @jakelandis this is welcome news, I'm curious on the details, perhaps we could sync up on the topic to learn of your current proposal? @seanstory is on vacation this week, but we can bring in @tarekziade !

@jakelandis
Copy link
Contributor Author

This has been completed via #87989

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Plugins Plugin API and infrastructure :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >enhancement Team:Core/Infra Meta label for core/infra team Team:Data Management Meta label for data/management team
Projects
None yet
Development

No branches or pull requests

3 participants