Try out constant_keyword value fields in system log integrations #1211

fearful-symmetry · 2021-06-24T21:51:30Z

What does this PR do?

This is a draft PR to test out the changes discussed here: https://github.com/elastic/security-team/issues/780
This is an addition based on the spec changes here: elastic/package-spec#194

This probably won't pass CI or anything now, as we're also waiting for this: elastic/elastic-package#386

This PR adds the value field to constant_keyword fields in the base yaml files.

Checklist

I have verified that all data streams collect metrics or logs.
I have added an entry to my package's changelog.yml file.
If I'm introducing a new feature, I have modified the Kibana version constraint in my package's manifest.yml file to point to the latest Elastic stack release (e.g. ^7.13.0).

elasticmachine · 2021-06-24T21:58:06Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Build Cause: Pull request #1211 updated
Start Time: 2021-06-29T16:21:33.555+0000
Duration: 11 min 13 sec
Commit: 64cac9f

Test stats 🧪

Test	Results
Failed	0
Passed	266
Skipped	0
Total	266

Trends 🧪

peluja1012 · 2021-06-24T23:39:37Z

Hi @fearful-symmetry, thanks for opening up this PR! From the Security Solution side, the most important piece is for this integration to populate both event.dataset and event.module. The idea, as described here, was that integrations, such as this one, could populate values for these fields by defining them as constant_keyword and then setting the value in the index mapping. I'm not too familiar with this codebase, but in this PR I don't see any changes related to event.dataset or event.module. If values for these fields are already being set somewhere else, then we're probably ok. But otherwise, we may need to make other updates here or somewhere else.

andrewkroh · 2021-06-25T13:53:14Z

Thanks for setting up a test. I merged support for the value field into Kibana so after we get some new snapshots I expect this should work. elastic/kibana#103000

fearful-symmetry · 2021-06-28T05:19:35Z

Figured I'd get the fields mixed up. Let's try that.

fearful-symmetry · 2021-06-28T05:22:41Z

@andrewkroh do we want to manually set the value for event.dataset or use some kind of alias?

ruflin · 2021-06-28T06:30:57Z

packages/system/data_stream/auth/fields/base-fields.yml

@@ -1,12 +1,22 @@
 - name: data_stream.type
  type: constant_keyword
  description: Data stream type.
+  value: logs


This is a nice addition we likely should add anywhere. @mtojek I remember we had some discussion around this in the past, maybe there is even an issue for it. Later on we should make sure it is in sync with the type in the manifest.

Are you referring to validation only? If so, then I understand that this property will ALWAYS be in sync with the type (any overriding forbidden)?

Yes, can't think of a use case where the two would not be in sync.

ruflin · 2021-06-28T06:35:26Z

packages/system/data_stream/auth/fields/base-fields.yml

 - name: data_stream.dataset
  type: constant_keyword
  description: Data stream dataset.
+  value: system.auth
+- name: event.dataset


Nit: Can we move event.dataset and event.module to the bottom to have it together as one block? Like this the diff is also cleaner to only have an addition on the bottom, assuming we leave out the changes for value in data_stream.*

mtojek

Please update the changelog and version in the package manifest.

CI complains about formatting and Git conflicts.

ruflin · 2021-06-28T08:15:26Z

For reference, here the two related issues: #226 elastic/kibana#66551

mtojek · 2021-06-28T12:56:59Z

packages/system/data_stream/security/fields/base-fields.yml

+  value: system.security
+- name: event.dataset
+  type: constant_keyword
+  description: event dataset.


nit: should be uppercase

mtojek · 2021-06-28T12:58:45Z

packages/system/data_stream/security/fields/base-fields.yml

+- name: event.dataset
+  type: constant_keyword
+  description: event dataset.
+  value: system.security
 - name: data_stream.namespace
  type: constant_keyword
  description: Data stream namespace.
 - name: dataset.type


@ruflin shouldn't this field be removed?

mtojek · 2021-06-28T12:58:59Z

packages/system/data_stream/security/fields/base-fields.yml

 - name: data_stream.namespace
  type: constant_keyword
  description: Data stream namespace.
 - name: dataset.type
  type: constant_keyword
  description: Dataset type.
+  value: logs
 - name: dataset.name


@ruflin shouldn't this field be removed? I think we replaced it with data_stream

mtojek · 2021-06-28T12:59:08Z

packages/system/data_stream/security/fields/base-fields.yml

 - name: dataset.name
  type: constant_keyword
  description: Dataset name.
+  value: system.security
 - name: dataset.namespace


@ruflin same here

…value_test

fearful-symmetry · 2021-06-28T16:10:08Z

Alright, everything should be updated now. Just depends on what we want to do with the value in the data_stream objects that I added because I was confused, as well as data_stream.type

fearful-symmetry · 2021-06-28T16:26:38Z

A tad confused by the CI failures, since ep check -v seems happy when I run it locally.

fearful-symmetry · 2021-06-28T16:51:42Z

...helps if I update elastic-package

peluja1012 · 2021-06-28T17:42:20Z

packages/system/changelog.yml

@@ -1,4 +1,9 @@
 # newer versions go on top
+- version: "0.13.5"
+  changes:
+    - description: Use event.datastream and event.module


Should this say event.dataset instead of event.datastream?

Good catch!

mtojek

Could you please make sure that dataset.* are not required anymore and clean them? I saw some in https://github.com/elastic/integrations/blob/27d37a116d765c32cccbbfd4f0773e563aed093c/packages/system/data_stream/security/fields/base-fields.yml .

fearful-symmetry · 2021-06-28T21:44:00Z

Could you please make sure that dataset.* are not required anymore and clean them?

@mtojek that's one of the things I'm not clear on. What uses dataset.*? I'm not really sure if it's safe to remove.

fearful-symmetry · 2021-06-29T07:35:52Z

Okay, so it just now occurred to me that when you said "required anymore" you were probably referring to the dashboards & saved searches. If that's the case, no, nothing is using them.

mtojek

LGTM

…value_test

try out constant_keyword values in system logs

9d023aa

fearful-symmetry added enhancement New feature or request v7.14.0 labels Jun 24, 2021

fearful-symmetry requested review from ycombinator and andrewkroh June 24, 2021 21:51

fearful-symmetry self-assigned this Jun 24, 2021

add fields wanted by security

a020a49

ruflin reviewed Jun 28, 2021

View reviewed changes

mtojek reviewed Jun 28, 2021

View reviewed changes

ycombinator removed their request for review June 28, 2021 14:10

update all data streams

2ddb0d3

fearful-symmetry marked this pull request as ready for review June 28, 2021 15:46

fearful-symmetry added 3 commits June 28, 2021 08:50

Merge remote-tracking branch 'upstream/master' into constant_keyword_…

0f58d85

…value_test

add changelog, fix security and docs

321e536

update version

dd5bfe1

fearful-symmetry requested a review from ruflin June 28, 2021 16:26

fearful-symmetry added 2 commits June 28, 2021 09:31

tinker with data_stream values

f2a5911

build again

00569e1

peluja1012 reviewed Jun 28, 2021

View reviewed changes

fix changelog

27d37a1

fearful-symmetry requested review from mtojek and peluja1012 June 28, 2021 20:29

mtojek reviewed Jun 28, 2021

View reviewed changes

remove datastream types

75b0af5

mtojek self-requested a review June 29, 2021 08:21

mtojek approved these changes Jun 29, 2021

View reviewed changes

fearful-symmetry added 2 commits June 29, 2021 09:17

Merge remote-tracking branch 'upstream/master' into constant_keyword_…

0bdaf9c

…value_test

update version

64cac9f

fearful-symmetry merged commit 52ae7e3 into elastic:master Jun 29, 2021

Try out constant_keyword value fields in system log integrations #1211

Try out constant_keyword value fields in system log integrations #1211

Uh oh!

Conversation

fearful-symmetry commented Jun 24, 2021

What does this PR do?

Checklist

Uh oh!

elasticmachine commented Jun 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

Uh oh!

peluja1012 commented Jun 24, 2021

Uh oh!

andrewkroh commented Jun 25, 2021

Uh oh!

fearful-symmetry commented Jun 28, 2021

Uh oh!

fearful-symmetry commented Jun 28, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtojek left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ruflin commented Jun 28, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fearful-symmetry commented Jun 28, 2021

Uh oh!

fearful-symmetry commented Jun 28, 2021

Uh oh!

fearful-symmetry commented Jun 28, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtojek left a comment

Choose a reason for hiding this comment

Uh oh!

fearful-symmetry commented Jun 28, 2021

Uh oh!

fearful-symmetry commented Jun 29, 2021

Uh oh!

mtojek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticmachine commented Jun 24, 2021 •

edited

Loading

mtojek left a comment •

edited

Loading