Skip to content
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

Honor reusable.top_level: false for generated ES templates #495

Closed
andrewkroh opened this issue Jul 2, 2019 · 5 comments
Closed

Honor reusable.top_level: false for generated ES templates #495

andrewkroh opened this issue Jul 2, 2019 · 5 comments
Labels
bug Something isn't working :codegen

Comments

@andrewkroh
Copy link
Member

When setting reusable.top_level: false the generated Elasticsearch template still includes the field in the template at the top level. I would not expect this field to be included at the top level of the template when this is false.

@webmat
Copy link
Contributor

webmat commented Apr 9, 2020

Most "reusable" field sets in ECS are only meant to be used in their nested location (e.g. source.geo, destination.geo, but not geo at the root of a document).

The documentation states this fact correctly, but many of the other generated artifacts still incorrectly declared the fields at the top level. As an example, the sample Elasticsearch templates do have field definitions for geo at the root of the document, and they shouldn't.

This issue actually affects the following artifacts:

  • Beats file for ECS field definitions
  • The sample Elasticsearch templates
  • The CSV file
  • The intermediate file ecs_flat.yml

I will be addressing this bug shortly for the last 3 artifacts.

In Beats, removing these fields may be seen as a breaking change. @andrewkroh or @leehinman could you let me know what your preference is, here? Happy to remove them in the Beats file, if you're confident that they were never used. For now I will leave them in, to avoid any surprises, but let me know if / when you want them removed, and I'll make it so, in this PR or another.

To complete the picture, here's a list of reusable field sets, and whether they are expected at the root of an ECS document (top_level: true/false in the YAML), as of ECS 1.5.0:

reusable field set expected at root
as no
code_signature no
geo no
group yes
hash no
interface no
os no
pe no
user yes
vlan no

webmat pushed a commit to webmat/ecs that referenced this issue Apr 9, 2020
This fixes elastic#495 for 'ecs_flat.yml', the csv and the Elasticsearch sample templates.
@andrewkroh
Copy link
Member Author

I can't guarantee they are not used, but I'm not aware of their usage in Beats. When they are removed we'll run automated tests that check that fields used in modules are in the template. We could run that test before we finalize the change in ECS (really just need the generated ECS fields.yml to drop into a beats PR to have the tests run).

@webmat
Copy link
Contributor

webmat commented Apr 10, 2020

Ok thanks. I'll still do these as separate PRs, I think, to avoid slowing down #813

But happy to provide you with the resulting Beats file today and get that merged quickly, if we're good on the Beats side.

@webmat
Copy link
Contributor

webmat commented Apr 10, 2020

I haven't opened a PR yet for it. Happy to fold into #813 if this works out well. But you can get the Beats file to test this from this branch fix-495-beats.

https://raw.githubusercontent.com/webmat/ecs/fix-495-beats/generated/beats/fields.ecs.yml

webmat pushed a commit that referenced this issue Apr 14, 2020
This commit removes field definitions from the root of a few artifacts, that should never have been present there (they have attribute top_level:false).

The artifacts affected are 'ecs_flat.yml', the csv and the Elasticsearch sample templates.
@webmat
Copy link
Contributor

webmat commented Apr 14, 2020

@andrewkroh #813 is merged.

Let me know if you'd like me to adjust the Beats artifact as well. I'll be happy to open a new PR for it.

@webmat webmat closed this as completed Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working :codegen
Projects
None yet
Development

No branches or pull requests

2 participants