Add orchestrator (resource.parent.type and resource.ip) and container (image.hash.all) fields #1889

daniel-almeida · 2022-04-20T22:11:36Z

Have you signed the contributor license agreement? yes
Have you followed the contributor guidelines? yes
For proposing substantial changes or additions to the schema, have you reviewed the RFC process? yes
If submitting code/script changes, have you verified all tests pass locally using make test? N/A
If submitting schema/fields updates, have you generated new artifacts by running make and committed those changes?yes
Is your pull request against main? Unless there is a good reason otherwise, we prefer pull requests against main and will backport as needed. yes
Have you added an entry to the CHANGELOG.next.md? yes

…ge.hash field.

ebeahan

Changes look good, @daniel-almeida. I just had a few thoughts about the naming.

Thanks for adding the changelog.next entry!

schemas/container.yml

schemas/orchestrator.yml

ebeahan

LGTM!

docs/fields/field-details.asciidoc

m-sample · 2022-04-26T20:13:43Z

docs/fields/field-details.asciidoc

+Note: this field should contain an array of values.
+


should be put a caveat comment for Pods. E.g. "In the case of Kubernetes Pods, there will be one IP address in this array"

This note was auto-generated. I updated the long description to cover that as well.

docs/fields/field-details.asciidoc

m-sample · 2022-04-26T20:22:42Z

docs/fields/field-details.asciidoc

+
+
+example: `DaemonSet`
+


ideally we would enumerate the values we currently expect along with whether they are case-sensitive etc. This will help sensor writers. Perhaps we could reference a Kube docs page for the list of 'nouns' and standard spelling & capitalization.

m-sample · 2022-04-26T20:31:25Z

experimental/generated/beats/fields.ecs.yml

+    - name: image.hash.sha256
+      level: extended


by having a separate field for each hash type (sha256, ....) we will need to update ECS for every new hash alg in use (e.g. "blake3" becomes popular next year). There may be an expectation that all of them are populated (ie we compute them vs just scooping the one from the Kube API).

Pardon the last minute observation on this but this approach seems very maintenance heavy (us) vs just having "<hashalg>:<b64 of hash>" as the value's structure for image.hash field. This approach keeps it to one field to look in and automatically supports new hash algs, and parsing isn't that hard - most folks will be looking for an equality match, I suspect so they could type "sha256:ab1234ace..." for their search.

I understand that perspective, and I'm fine if we decide on image.hash.

My thinking around container.image.hash.sha256:

Follow the naming convention used by the hash.* field set and its reuses, like file.hash.sha256.

If the related.* fields are being populated, related.hash helps simplify searching across the different hash fields.

Avoid establishing image.hash as a leaf field

interesting. related.hash as an array of all the hash values doesn't seem to help as there would only be 1 entry in the array and it wouldn't be self-describing (could be sha256, sha512, ?).

I agree we should not overload "hash" to be a the existing field set or this new primitive valued one :head-explode:.

How does this sound: Add a field to the "hash" field set, something like hash.all that holds an array of self-describing hash values like <hash-alg>:<b64-hash-bytes>. Sort of related.hash but co-located with hash field set and self-describing. This would accommodate the current case of 1 hash value read from the kube API, as well as possible future use cases where we populate the different hash algs values too. Allowed values for the hash-alg part would be the existing field names in the hash fieldset? Thankfully they seem to align with Kubernetes use of "sha256" (same as ECS field name).

Mike, just to clarify: you're suggesting both fields (hash.sha256 and hash.all) should be set by producers, right?

Just hash.all to start. Rationale: keep our events DRY (no duplicated information in an event) to save on bandwidth and storage.

Works for me 👍

I think we meant to say container.image... instead of orchestrator.image.... Just pushed that change; let me know if it's good to go.

@ebeahan , I noticed that hash values in the TLS field set you referenced are capitalized for consistency. Should we do the same here?

@daniel-almeida, ECS field names are always lowercase. Is that what you mean?

No, I meant the [hash] values. If you look at https://www.elastic.co/guide/en/ecs/current/ecs-tls.html#field-tls-client-hash-sha256, you will notice that the example value is capitalized and that there's a note about it.

m-sample

LGTM

m-sample · 2022-04-30T00:22:42Z

generated/beats/fields.ecs.yml

+      type: keyword
+      ignore_above: 1024
+      description: 'An array of digests of the image the container was built on. Each
+        digest consists of the hash algorithm and value in this format: `algorithm:value`.'


I would just add "Algorithm names in this value should align with the field names in the ECS hash field set" if that makes sense.

ebeahan

LGTM

… (image.hash.all) fields (elastic#1889) * Add orchestrator.resource parent_type and ip fields and container.image.hash field. * Add generated artifacts. * Update CHANGELOG. * Update definition of container.image.hash * Address PR comments. * Address PR comments. * Add container.image.hash.all * address PR comment * Update CHANGELOG.next (cherry picked from commit 3846878) # Conflicts: # experimental/generated/csv/fields.csv # generated/csv/fields.csv

ebeahan · 2022-05-02T18:45:38Z

💚 All backports created successfully

Status	Branch	Result
✅	8.3

Questions ?

Please refer to the Backport tool documentation

… (image.hash.all) fields (#1889) (#1896) * Add orchestrator.resource parent_type and ip fields and container.image.hash field. * Add generated artifacts. * Update CHANGELOG. * Update definition of container.image.hash * Address PR comments. * Address PR comments. * Add container.image.hash.all * address PR comment * Update CHANGELOG.next (cherry picked from commit 3846878) Co-authored-by: Daniel Araujo Almeida <[email protected]>

daniel-almeida added 2 commits April 20, 2022 16:08

Add orchestrator.resource parent_type and ip fields and container.ima…

d3a8fb6

…ge.hash field.

Add generated artifacts.

a2c50b6

daniel-almeida added the 8.3.0 label Apr 20, 2022

Update CHANGELOG.

3bff102

daniel-almeida marked this pull request as ready for review April 21, 2022 15:14

daniel-almeida requested a review from a team as a code owner April 21, 2022 15:14

ebeahan reviewed Apr 21, 2022

View reviewed changes

schemas/container.yml Outdated Show resolved Hide resolved

schemas/orchestrator.yml Outdated Show resolved Hide resolved

schemas/orchestrator.yml Outdated Show resolved Hide resolved

schemas/orchestrator.yml Outdated Show resolved Hide resolved

daniel-almeida added 2 commits April 21, 2022 21:36

Update definition of container.image.hash

0b697f1

Address PR comments.

872c892

daniel-almeida requested a review from ebeahan April 25, 2022 18:00

ebeahan approved these changes Apr 26, 2022

View reviewed changes

m-sample reviewed Apr 26, 2022

View reviewed changes

docs/fields/field-details.asciidoc Outdated Show resolved Hide resolved

m-sample reviewed Apr 26, 2022

View reviewed changes

docs/fields/field-details.asciidoc Outdated Show resolved Hide resolved

m-sample reviewed Apr 26, 2022

View reviewed changes

daniel-almeida added 2 commits April 26, 2022 19:29

Address PR comments.

fd19c5b

Add container.image.hash.all

bfb4924

daniel-almeida requested review from ebeahan and m-sample April 29, 2022 21:13

m-sample approved these changes Apr 30, 2022

View reviewed changes

address PR comment

06f549d

daniel-almeida changed the title ~~Add orchestrator (resource.parent_type and resource.ip) and container (container.image.hash) fields~~ Add orchestrator (resource.parent.type and resource.ip) and container (container.image.hash.all) fields May 2, 2022

Update CHANGELOG.next

3174d72

ebeahan approved these changes May 2, 2022

View reviewed changes

daniel-almeida changed the title ~~Add orchestrator (resource.parent.type and resource.ip) and container (container.image.hash.all) fields~~ Add orchestrator (resource.parent.type and resource.ip) and container (image.hash.all) fields May 2, 2022

daniel-almeida merged commit 3846878 into main May 2, 2022

ebeahan added the needs_backport label May 2, 2022

ebeahan mentioned this pull request May 2, 2022

[8.3] Add orchestrator (resource.parent.type and resource.ip) and container (image.hash.all) fields (#1889) #1896

Merged

ebeahan removed the needs_backport label May 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add orchestrator (resource.parent.type and resource.ip) and container (image.hash.all) fields #1889

Add orchestrator (resource.parent.type and resource.ip) and container (image.hash.all) fields #1889

daniel-almeida commented Apr 20, 2022 •

edited

Loading

ebeahan left a comment

ebeahan left a comment

m-sample Apr 26, 2022

daniel-almeida Apr 27, 2022

m-sample Apr 26, 2022

m-sample Apr 26, 2022 •

edited

Loading

ebeahan Apr 26, 2022

m-sample Apr 26, 2022 •

edited

Loading

daniel-almeida Apr 27, 2022

m-sample Apr 27, 2022 •

edited

Loading

ebeahan Apr 29, 2022

daniel-almeida Apr 29, 2022

daniel-almeida Apr 30, 2022

ebeahan May 2, 2022

daniel-almeida May 2, 2022

m-sample left a comment

m-sample Apr 30, 2022

ebeahan left a comment

ebeahan commented May 2, 2022



		example: `DaemonSet`

Add orchestrator (resource.parent.type and resource.ip) and container (image.hash.all) fields #1889

Add orchestrator (resource.parent.type and resource.ip) and container (image.hash.all) fields #1889

Conversation

daniel-almeida commented Apr 20, 2022 • edited Loading

ebeahan left a comment

Choose a reason for hiding this comment

ebeahan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-sample Apr 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-sample Apr 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-sample Apr 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-sample left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebeahan left a comment

Choose a reason for hiding this comment

ebeahan commented May 2, 2022

💚 All backports created successfully

Questions ?

daniel-almeida commented Apr 20, 2022 •

edited

Loading

m-sample Apr 26, 2022 •

edited

Loading

m-sample Apr 26, 2022 •

edited

Loading

m-sample Apr 27, 2022 •

edited

Loading