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

Fix for issue #8425 ti_opencti integration causes field conflict #8428

Merged
merged 5 commits into from
Nov 24, 2023

Conversation

colin-stubbs
Copy link
Contributor

  • Bug

Proposed commit message

Added primary ingest pipeline processor to ensure event.original is removed if the preserve_original_event tag is NOT present.

Added primary ingest pipeline processor to ensure event.original is converted to a text field if it exists, to support storage as a text keyword field as per ECS definition.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • elastic-package build completed without errors
  • elastic-package check completed without errors
  • elastic-package test --generate completed without errors

How to test this PR locally

Deploy and test locally using elastic-package based stack.

Related issues

Screenshots

Not applicable.

@colin-stubbs colin-stubbs requested a review from a team as a code owner November 8, 2023 02:10
@elasticmachine
Copy link

elasticmachine commented Nov 8, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-11-24T03:32:30.936+0000

  • Duration: 15 min 46 sec

Test stats 🧪

Test Results
Failed 0
Passed 22
Skipped 0
Total 22

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@chrisberkhout
Copy link
Contributor

chrisberkhout commented Nov 8, 2023

Hi Colin,

Thanks for the PR!

Turning on preserve_original_event for the system tests and having that show in the sample event and documentation is good.

Currently, event.original should only be added if preserve_original_event is set, so I think removing it otherwise is redundant. Note that it's the preserve_original_event setting that determines adding the field and the tag, rather the field addition/retention being based on a tag added in another way.

I notice that the string conversion step is generating a stringified object that isn't in JSON format:
{standard_id=indicator--cde0a6e1-c622-52c4-b857-e9aeac56131b, ...} vs
{"standard_id":"indicator--cde0a6e1-c622-52c4-b857-e9aeac56131b", ...}.
I'm inclined to stringify to valid JSON where the field is inserted, in the CEL expression, like this:

diff --git a/packages/ti_opencti/data_stream/indicator/agent/stream/cel.yml.hbs b/packages/ti_opencti/data_stream/indicator/agent/stream/cel.yml.hbs
index 3f6d08388..f07a0025c 100644
--- a/packages/ti_opencti/data_stream/indicator/agent/stream/cel.yml.hbs
+++ b/packages/ti_opencti/data_stream/indicator/agent/stream/cel.yml.hbs
@@ -53,7 +53,7 @@ program: |
     bytes(resp.Body).decode_json().as(body, state.with({
       "events": body.data.indicators.edges.map(e, e.node.with(
         has(state.preserve_original_event) && state.preserve_original_event ?
-          { "event": { "original": e.node } } :
+          { "event": { "original": e.node.encode_json() } } :
           {}
       )),
       "want_more": body.data.indicators.pageInfo.hasNextPage,

What do you think?

@colin-stubbs
Copy link
Contributor Author

colin-stubbs commented Nov 9, 2023

Ah yes! I didn't actually double check the result in event.original :-(

Yeap, doing it there instead also makes sense, zero issues.

I'll adjust the PR.

@bhapas
Copy link
Contributor

bhapas commented Nov 10, 2023

/test

@chrisberkhout
Copy link
Contributor

To make the tests pass, run elastic-package build and commit the updated packages/ti_opencti/docs/README.md.
(docs/README.md is generated from _dev/build/docs/README.md using Handlebars templating).

@colin-stubbs
Copy link
Contributor Author

rebuild done

@chrisberkhout
Copy link
Contributor

@colin-stubbs One more thing: there are conflicts to resolve, since a couple of other changes went in since the PR was opened.

@elasticmachine
Copy link

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 50.0% (5/10) 👎 -44.444
Classes 50.0% (5/10) 👎 -44.444
Methods 59.615% (31/52) 👎 -30.086
Lines 83.25% (666/800) 👎 -2.833
Conditionals 100.0% (0/0) 💚

@chrisberkhout chrisberkhout merged commit 9751ae0 into elastic:main Nov 24, 2023
3 checks passed
@elasticmachine
Copy link

Package ti_opencti - 0.3.1 containing this change is available at https://epr.elastic.co/search?package=ti_opencti

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ti_opencti integration causes field conflict
5 participants