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

feat: include alias path when generating template #877

Merged
merged 12 commits into from
Oct 16, 2020
Merged

feat: include alias path when generating template #877

merged 12 commits into from
Oct 16, 2020

Conversation

rgmz
Copy link
Contributor

@rgmz rgmz commented Jun 25, 2020

This fixes #876

Done quickly via GitHub web, but let me know if there are any additional changes required (e.g. tests or changelog).

@cla-checker-service
Copy link

cla-checker-service bot commented Jun 25, 2020

💚 CLA has been signed

@rgmz
Copy link
Contributor Author

rgmz commented Jun 25, 2020

x Author of the following commits did not sign a Contributor Agreement:
9864f19

Please, read and sign the above mentioned agreement if you want to contribute to this project

I signed the CLA prior to creating this PR, so maybe it takes a bit of time to be recognized.

@webmat webmat added the review label Jun 25, 2020
@webmat
Copy link
Contributor

webmat commented Jun 25, 2020

I re-ran our tool to check the CLA, and you signed it correctly indeed.

Thanks for submitting this, we will review shortly

@ebeahan
Copy link
Member

ebeahan commented Jul 1, 2020

Thanks again for submitting @rgmz! I see value in adding support for the alias type per your description in #876 .

These changes would introduce an optional path field set attribute to the schema, which is required when type is specified as alias? Is the following example snippet an accurate example?

    - name: id
      level: extended
      type: alias
      path: custom.host.id

We'll need to consider other aspects for supporting this additional path field and the alias type (likely others - these were two that came to mind so far):

  • The beats generator, ./scripts/generators/beats.py should also support path when type is alias.
  • The target of path is subject to restrictions which wouldn't be checked or enforced by the ECS tooling (at least initially). We could mark the support for alias as experimental and note the limitation, but it could cause some hidden complexity when attempting to apply the mappings.

@webmat
Copy link
Contributor

webmat commented Jul 2, 2020

Yeah I support this addition.

This only fixes it for the Go generator. I'd like it if we failed the run if path is missing on an alias field, in cleaner.py / field_mandatory_attributes. Check it out here

@rgmz Are you good with adding this? If not, perhaps @ebeahan could pick this up?

@ebeahan
Copy link
Member

ebeahan commented Jul 2, 2020

This only fixes it for the Go generator

ES generator? Or is there a relationship to the Go generator I'm overlooking?

I did see a possible need for introducing a new type in the Go generator, but since how we're aiming to remove it (#628) in the near future I wasn't concerned. I also did some light testing of the branch and didn't see any immediate issues.

@webmat
Copy link
Contributor

webmat commented Jul 2, 2020

Yes the alias datatype itself should make it there no problem. But it's a good point to ensure the path attribute makes it there too, and that it's not being filtered out.

Note that ECS doesn't make use of the alias type, so we should make sure the ES script has test coverage for this as well.

@ebeahan ebeahan self-assigned this Aug 4, 2020
@ebeahan ebeahan added the ready Issues we'd like to address in the future. label Aug 4, 2020
@rgmz
Copy link
Contributor Author

rgmz commented Oct 4, 2020

@webmat @ebeahan

Apologies for letting this stagnate, it's been a busy few months! :)

These changes would introduce an optional path field set attribute to the schema, which is required when type is specified as alias? Is the following example snippet an accurate example?

    - name: id
      level: extended
      type: alias
      path: custom.host.id

Correct. The intention is to match the schema to the Elasticsearch usage of alias, which you've linked.

We'll need to consider other aspects for supporting this additional path field and the alias type (likely others - these were two that came to mind so far):

  • The beats generator, ./scripts/generators/beats.py should also support path when type is alias.

  • The target of path is subject to restrictions which wouldn't be checked or enforced by the ECS tooling (at least initially). We could mark the support for alias as experimental and note the limitation, but it could cause some hidden complexity when attempting to apply the mappings.

I've updated ./scripts/generators/beats.py and scripts/schema/cleaner.py to support path for the alias type.

I agree that validating the generated artifacts would be ideal, however, that's probably out of scope for this PR. e.g., you can also generate an invalid template by defining type: invalid_type123.

Copy link
Member

@ebeahan ebeahan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the continue work here @rgmz ! This will be a nice feature addition.

Outside of the few notes I made below, can you also add:

  • A brief entry in the Supported keys to describe fields section of schemas/README.md for path
  • A changelog entry to CHANGELOG.next.md

scripts/generators/es_template.py Show resolved Hide resolved
scripts/generators/beats.py Outdated Show resolved Hide resolved
scripts/generators/beats.py Outdated Show resolved Hide resolved
@rgmz
Copy link
Contributor Author

rgmz commented Oct 13, 2020

Thanks for the continue work here @rgmz ! This will be a nice feature addition.

Outside of the few notes I made below, can you also add:

* [ ]  A brief entry in the `Supported keys to describe fields` section of  `schemas/README.md` for `path`

* [ ]  A changelog entry to `CHANGELOG.next.md`

Done!

(Working from the GitHub UI once again, so let me know if I missed anything.)

Throwing an exception during generation was removed in a prior commit, so this test is no longer relevant.
Copy link
Member

@ebeahan ebeahan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great @rgmz ! Thanks for making those changes.

I'm approving and will handle merging and then backporting 🚢

@ebeahan ebeahan merged commit 78c8549 into elastic:master Oct 16, 2020
ebeahan pushed a commit to ebeahan/ecs that referenced this pull request Oct 16, 2020
ebeahan pushed a commit to ebeahan/ecs that referenced this pull request Oct 16, 2020
ebeahan added a commit that referenced this pull request Oct 16, 2020
@webmat
Copy link
Contributor

webmat commented Oct 19, 2020

Thanks @rgmz and @ebeahan for getting this to the finish line :-)

@rgmz rgmz deleted the patch-1 branch January 28, 2021 15:57
dseeley added a commit to dseeley/ecs that referenced this pull request May 5, 2021
* bumping version for 1.x release branch (elastic#921)

* [1.x] add related.hosts (elastic#913) (elastic#924)

* [1.x][DOCS] Fixes SIEM links (elastic#936)

* [1.x] Consolidate field-details doc template (elastic#897) (elastic#946)

* Add http.[request|response].mime_type (elastic#944) (elastic#949)

* [1.x] Cut 1.6 Changelog (elastic#933) (elastic#952) (elastic#953)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add threat.technique.subtechnique (elastic#951) (elastic#956)

Co-authored-by: Ross Wolf <[email protected]>

* [1.x] Nest as for foreign reuse (elastic#960) (elastic#962)

* [1.x] Remove `expected_event_types` from protocol (elastic#964) (elastic#965)

* [1.x] Expand definitions of source and destination field sets (elastic#967) (elastic#973)

* [1.x] Introduce `--strict` flag (elastic#937) (elastic#975)

* [1.x] Add example value composite type checking (elastic#966) (elastic#976)

* Add example value composite type checking (elastic#966)
* generate csv artifact

* [1.x] Add event category configuration (elastic#963) (elastic#977)

* [1.x] Add normalizer multi-field capability (elastic#971) (elastic#978)

Co-authored-by: Eric Beahan <[email protected]>

Co-authored-by: Madison Caldwell <[email protected]>

* [1.x] Add mapping network event guidance doc (elastic#969) (elastic#983)

* [1.x] Removing unneeded link under `Additional Information` (elastic#984) (elastic#985)

* [1.x] Add discrete attribute to field details page headers (elastic#989) (elastic#990)

* [1.x] Uniformity across domain name breakdown fields (elastic#981) (elastic#994)

Co-authored-by: Mathieu Martin <[email protected]>

* Add --oss flag to the ECS generator script (elastic#991) (elastic#995)

* Add network directions ingress and egress (elastic#945) (elastic#997)

* Mention ECS Mapper in the main documentation (elastic#987) (elastic#1000)

Co-authored-by: Dan Roscigno <[email protected]>

* [1.x] Introduce experimental artifacts (elastic#993) (elastic#1001)

Co-authored-by: Mathieu Martin <[email protected]>

* Bump version to 1.8.0-dev in branch 1.x (elastic#1011)

* Cut 1.7 changelog (elastic#1010) (elastic#1012)

* [1.x] Clarify that file extension should exclude the dot. (elastic#1016) (elastic#1020)

* [1.x] Add usage docs section (elastic#988) (elastic#1024)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] feat: include alias path when generating template (elastic#877) (elastic#1035)

Co-authored-by: Richard Gomez <[email protected]>

* [1.x] Add support for `scaling_factor` in the generator (elastic#1042) (elastic#1055)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add fallback for constant_keyword (elastic#1046) (elastic#1056)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add wildcard type support to go code generator (elastic#1050) (elastic#1057)

* add wildcard type support

* also add version and constant_keyword

* changelog

* [1.x] New default make task that generates main and experimental artifacts. (elastic#1041) (elastic#1060)

Also changing the order of the 'generate' task: it now starts with the new generator, then runs the legacy scripts.

* [1.x] Change the index pattern in the sample template. (elastic#1048) (elastic#1068)

* [1.x] Prepare link to Logs docs changing with the 7.10 release in "getting-started" (elastic#1073) (elastic#1079)

Co-authored-by: EamonnTP <[email protected]>

* [1.x] Prepare link to Logs docs changing with the 7.10 release in "products-solutions" page (elastic#1074) (elastic#1083)

Co-authored-by: EamonnTP <[email protected]>

* [1.x] Add event.category session. (elastic#1049) (elastic#1093)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add event.category registry (elastic#1040) (elastic#1094)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add --ref support for experimental artifacts (elastic#1063) (elastic#1101)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Remove experimental event.original definition (elastic#1053) (elastic#1104)

* [1.x] Add missing `process.thread.name` to experimental definitions (elastic#1103) (elastic#1106)

* [1.x] Remove index parameter for wildcard fields (elastic#1115) (elastic#1119)

* [1.x] Add dns.answer object into experimental schema (elastic#1118) (elastic#1121)

* [1.x] Clarify x509 definition guidance for network events with only one cert (elastic#1114) (elastic#1123)

* [1.x] Indicate when artifacts include experimental changes (elastic#1117) (elastic#1125)

* [1.x] Add os.type field, with list of allowed values (elastic#1111) (elastic#1130)

* [1.x] Add support for constant_keyword's 'value' parameter (elastic#1112) (elastic#1132)

* [1.x] Beta label support (elastic#1051) (elastic#1133)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Backport elastic#1134 and elastic#1135 (elastic#1136)

* Remove temporary ifeval in "getting started" page, add link to Metrics docs (elastic#1134)
* Remove temporary ifeval from products page, add link to Metrics (elastic#1135)

* Two small documentation backports (elastic#1149)

* Remove an incorrect `event.type` from the 'converting' page (elastic#1146)
* Mention Logstash support for ECS in the 'products' page (elastic#1147)

* [1.x] Reinforce the exclusion of the leading dot from url.extension (elastic#1151) (elastic#1152)

* [1.x] Make all fields linkable directly via an HTML ID (elastic#1148) (elastic#1154)

* [1.x] Tracing fields should be at the root (elastic#1165)

* Add notice to the tracing field set, about not nesting field names. (elastic#1162)
* Tracing fields should be at top level in Beats artifact (elastic#1164)

* [1.x] Usage of brackets for a URL containing IPv6 address (elastic#1131) (elastic#1168)

* [1.x] 6.x index template data type fallback (elastic#1171) (elastic#1172)

* [1.x] Apply RFC 0007 stage 3 changes - multi-user (elastic#1066) (elastic#1175)

Conflict: deleted file rfcs/text/0007-multiple-users.md as RFCs are not backported to version branches.

* [1.x] Handle `error.stack_trace` case for ES 6.x template (elastic#1176) (elastic#1177)

* [1.x] Add composable index templates artifacts (elastic#1156) (elastic#1179)

* [1.x] Move _meta section back inside mappings, in legacy templates. (elastic#1186) (elastic#1187)

Backports the following commits to 1.x:

* Move _meta section back inside mappings, in legacy templates. (elastic#1186) 

This fixes an issue introduced by elastic#1156, discovered in elastic#1180. Composable templates support `_meta` at the template's root, but legacy templates don't. So we're just putting it back inside the mappings for legacy templates.

This also fixes missing updates to the component template, after the introduction of wildcard in elastic#1098.

* [1.x] Apply the RFC 0005 stage 2 (host metrics) changes in the experimental artifacts (elastic#1159) (elastic#1184)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Stage 3 changes for wildcard RFC 0001 (elastic#1098) (elastic#1183)

* [1.x] Conditional handling in es_template.template_settings (elastic#1191) (elastic#1192)

* [1.x] Artifacts docs page (elastic#1189) (elastic#1195)

* [1.x] Remove beta warning label from categorization fields docs (elastic#1067) (elastic#1196)

* [1.x] Correct wording of `event.reference` description (elastic#1181) (elastic#1197)

* Bump version to 1.9.0-dev in branch 1.x (elastic#1198)

* [1.x] Cut 1.8 FF changelog.next.md elastic#1199 (elastic#1201)

* Merge custom and core multi_fields arrays (elastic#982) (elastic#1213)

Co-authored-by: Jonathan Buttner <[email protected]>

* [1.x] Stage 2 changes for RFC 0009 - data_stream fields (elastic#1215) (elastic#1222)

* [1.x] add http.request.id (elastic#1208) (elastic#1223)

Co-authored-by: Eric Beahan <[email protected]>
Co-authored-by: Gil Raphaelli <[email protected]>

* [1.x] add cloud.service.name (elastic#1204) (elastic#1224)

* add cloud.platform

* expand cloud.platform description

* move to cloud.service.name

Co-authored-by: Gil Raphaelli <[email protected]>

* [1.x] Add ssdeep hash (elastic#1169) (elastic#1227)

Co-authored-by: Andrew Stucki <[email protected]>

* [CI] Switch to GitHub actions (elastic#1236) (elastic#1245)

Co-authored-by: Eric Beahan <[email protected]>

Co-authored-by: Andrew Stucki <[email protected]>

* Revert wildcard adoption back to experimental stage (elastic#1235) (elastic#1243)

* Add scaled_float type to go generator (elastic#1250) (elastic#1251)

* add scaled_float

* changelog

* Add categorization fields usage docs (elastic#1242) (elastic#1257)

* add time_zone, postal_code, and continent_code (elastic#1229) (elastic#1258)

* Specify MAC address format (elastic#456) (elastic#1260)

Co-authored-by: Robin Schneider <[email protected]>

* finalize 1.8.0 changelog (elastic#1262) (elastic#1265)

* Add additional host fields (elastic#1248) (elastic#1267)

Co-authored-by: kaiyan-sheng <[email protected]>

* Stage 1 changes for RFC 0014 - extend pe fields (elastic#1256) (elastic#1270)

* Add 2 fields to code_signature (elastic#1269) (elastic#1272)

Co-authored-by: Yamin Tian <[email protected]>

* Stage 3 changes for RFC 0007 - remove beta attribute (elastic#1271) (elastic#1273)

* Stage 1 experimental changes for RFC 0008 - threat.indicator fields (elastic#1268) (elastic#1274)

* Stage 1 changes for RFC 0015 - add elf fieldset (elastic#1261) (elastic#1275)

* Cut 1.9 FF CHANGELOG.next.md (elastic#1277)

* lock go version in actions (elastic#1283) (elastic#1290)

* Bump jinja2 from 2.11.2 to 2.11.3 in /scripts (elastic#1310) (elastic#1320)

* Bump jinja2 from 2.11.2 to 2.11.3 in /scripts

* Bump pyyaml from 5.3b1 to 5.4 in /scripts (elastic#1318) (elastic#1325)

Co-authored-by: Eric Beahan <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Adjust terminology - change whitelist to allowlist (elastic#1315) (elastic#1331)

Co-authored-by: Dominic Page <[email protected]>

* Remove -dev label from 1.9 version (elastic#1329)

* remove -dev label from 1.9 version

* generate artifacts

* removing rules artifacts

* Cut 1.9 changelog (elastic#1328)

* move 1.9 changes to changelog

* add 1.9 release changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Issues we'd like to address in the future. review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add generator support for alias datatype
3 participants