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

[TI_Anomali] Add a field to easily filter transform's source and destination indices #6344

Merged
merged 7 commits into from May 31, 2023

Conversation

kcreddy
Copy link
Contributor

@kcreddy kcreddy commented May 26, 2023

What does this PR do?

Bakground:
Adding indicator rules is a bit complex after IOC expiration support is launched. The destination indices (unexpired indicators) are needed to be queried using _index field and also using explicitly added Recorded Future and Anomali rules separately.

This PR:

  • Adds a field onto source ingest pipeline as a placeholder separating source and destination indices.
  • This field is deleted inside destination ingest pipeline run by the transform.

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

  • [ ]

How to test this PR locally

Related issues

Screenshots

@kcreddy kcreddy added enhancement New feature or request Integration:Anomali labels May 26, 2023
@elasticmachine
Copy link

elasticmachine commented May 26, 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-05-31T11:25:43.232+0000

  • Duration: 16 min 31 sec

Test stats 🧪

Test Results
Failed 0
Passed 11
Skipped 0
Total 11

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

# This field is deleted inside destination ingest pipeline run by the transform.
# The field is added to efficiently filter out destination indices containing unexpired indicators.
- set:
field: is_ioc_transform_source
Copy link
Member

Choose a reason for hiding this comment

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

The ECS labels.* field could be a good place for this data. The value should be a string to use labels.

https://www.elastic.co/guide/en/ecs/8.5/ecs-base.html#field-labels

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added labels ECS field, but also had to define inside fields.yml due to errors.
More details here - #6344 (comment)

@elasticmachine
Copy link

elasticmachine commented May 26, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 100.0% (1/1) 💚
Classes 100.0% (1/1) 💚
Methods 100.0% (13/13) 💚
Lines 95.48% (338/354)
Conditionals 100.0% (0/0) 💚

@kcreddy kcreddy self-assigned this May 26, 2023
@kcreddy kcreddy marked this pull request as ready for review May 26, 2023 17:18
@kcreddy kcreddy requested a review from a team as a code owner May 26, 2023 17:18
@elasticmachine
Copy link

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

Could we accomplish this with a constant_keyword field?

field: error.message
value: '{{{_ingest.on_failure_message}}}'
- append:
field: event.kind
Copy link
Member

Choose a reason for hiding this comment

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

AFAIK event.kind is a scalar in ECS. Using append is incompatible with the type. How about a set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used set processor. I also noticed quite a few integrations have been using append processor which probably needs fixing as well.

Copy link
Member

Choose a reason for hiding this comment

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

Is it required to increment fleet_transform_version in order to have this new feature installed on upgrade?

Copy link
Contributor Author

@kcreddy kcreddy May 31, 2023

Choose a reason for hiding this comment

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

Removed the destination index pipeline since constant_keyword with default value of true worked. So, doesn't require any transform changes. But yeah it was a required change if we went with adding destination ingest pipeline

@@ -100,3 +100,6 @@
type: date
description: >-
Date when IOC was deleted/expired.
- name: labels.is_ioc_transform_source
type: keyword
Copy link
Member

Choose a reason for hiding this comment

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

What about if this became a constant_keyword field with a static value? Then we would not need to modify the ingest pipeline to add it. Consequently we would not need to add a transform pipeline to remove the value because it would not be present in _source.

Suggested change
type: keyword
type: constant_keyword
value: "true"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the field with constant_keyword and removed destination ingest pipeline. No change to transform is required and verified that the field is not present in the destination index.

@kcreddy kcreddy merged commit 470af81 into elastic:main May 31, 2023
3 checks passed
@elasticmachine
Copy link

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

agithomas pushed a commit to agithomas/integrations that referenced this pull request Jun 5, 2023
…ination indices (elastic#6344)

* Add field for seperating ioc indices

* update pr num

* Address PR comment., Add to labels

* update README

* update package version prefix

* update tests

* Remove dest pipeline
@kcreddy kcreddy deleted the anomali_ioc_field branch June 8, 2023 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants