-
Notifications
You must be signed in to change notification settings - Fork 514
Resolve #11741, bbot update based on testing using BBOT v2.1.2 #11742
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
Conversation
packages/bbot/_dev/deploy/docker/sample_logs/bbot-v1-webhook.log
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there specific fields that need to be included here to prevent mismapping? If not, this does not need to be present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My experience is that there is a bunch of base ECS fields that need to be explicitly defined as elastic-package doesn't know about them for whatever reason. I'll re-test and dump the output if it's still the case with v0.108.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... nah, this isn't even that issue.
These need to be defined here for the field types to explicitly make it into the index component template. Otherwise yes 100% guaranteed some type of field type conflict will occur between the fields in the bbot.asm_intel indices and some other indices that have ECS conformant field types.
Expecting dynamic mapping by Elasticsearch to work reliably across all indices with highly variable data is, in my experience, a guaranteed way to get type conflicts and search failures.
related.ip is the most obvious one in my experience, as without explicit definition here Elasticsearch will simply consider it as a keyword within bbot.asm_intel indices while it will actually be defined as IP somewhere else.
url.full and url.original can also be problematic depending on how long/large they are in the early stages of dynamic mapping they can go either way as keyword or as text.
I'm happy to be corrected wrong if this has somehow been fixed in 8.17.x etc, I just strongly doubt it's the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I think I see what you're getting at... most were not yet mapped in the pipelines, so explicit definition really not needed (yet). I'll improve the mapping and clean this up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What @efd6 means by this is you don't need this file as long as your ECS fields adhere to standard. If you need to override a ECS field mapping, for example, you want to map event.code as an integer instead of standard keyword, then you only need to add those fields in this file.
Context: Starting from 8.13, we have standard ecs@mappings component template which automatically takes care of mapping all standard ECS fields, without needing to manually define in ecs.yml. Example PR: #10135 removes ecs.yml as we don't need them from 8.13.
From your fields list, it seems all of them are defined as - external: ecs and hence they are standard ECS mappings. You don't probably need this file.
packages/bbot/data_stream/asm_intel/elasticsearch/ingest_pipeline/ecs.yml
Outdated
Show resolved
Hide resolved
packages/bbot/data_stream/asm_intel/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/bbot/data_stream/asm_intel/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/bbot/data_stream/asm_intel/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
|
Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as |
|
@efd6 @andrewkroh how do we get this moving now? |
|
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-reviewed @efd6 review comments.
Unnecessary ECS field mappings is the one that stands out: #11742 (comment).
packages/bbot/data_stream/asm_intel/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What @efd6 means by this is you don't need this file as long as your ECS fields adhere to standard. If you need to override a ECS field mapping, for example, you want to map event.code as an integer instead of standard keyword, then you only need to add those fields in this file.
Context: Starting from 8.13, we have standard ecs@mappings component template which automatically takes care of mapping all standard ECS fields, without needing to manually define in ecs.yml. Example PR: #10135 removes ecs.yml as we don't need them from 8.13.
From your fields list, it seems all of them are defined as - external: ecs and hence they are standard ECS mappings. You don't probably need this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI has following error as pipeline tests are failing:
pipeline test: test-bbot-v2-ndjson.log in bbot.asm_intel
test case failed: Expected results are different from actual ones: --- want
+++ got
@@ -4591,6 +4591,7 @@
],
"url": {
"domain": "example.com",
+ "extension": "well-known/openid-configuration",
"full": [
"https://login.windows.net/example.com/.well-known/openid-configuration"
],
You can follow this PR: #9623 to add below block to pipeline test config to prevent error in url.extension:
dynamic_fields:
# This can be removed after ES 8.14 is the minimum version.
# Relates: https://github.com/elastic/elasticsearch/pull/105689
url.extension: '^.*$'
|
@kcreddy thank you! I've gotten up to speed with some of these changes to the index templating and ingest pipeline processes that are attempting to ensure ECS conformance. Who would be best to suggest add some content to https://www.elastic.co/guide/en/integrations-developer/current/index.html with a visualisation similar to the below? Obviously a better one than my mud map, and ideally created by someone who really understands how this all hangs together a little better. I think this would help explain, more completely and visually, what the current state of things is to anyone developing integrations. |
|
/test |
@colin-stubbs, the current documentation resides in here: https://github.com/elastic/observability-docs/tree/main/docs/en/integrations cc: @bmorelli25 who recently updated the doc: elastic/observability-docs#3875 |
🚀 Benchmarks reportTo see the full report comment with |
💚 Build Succeeded
History
|
|
kcreddy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏼
|
Package bbot - 1.2.0 containing this change is available at https://epr.elastic.co/package/bbot/1.2.0/ |
|
woohoo! @colin-stubbs we've been following your PR - excited to try this out! |
|
Congrats!! |
…elastic#11742) This commit adds BBOT v2.x NDJSON format support the integration, while simultaneously supporting the existing BBOT v1.x format. It also adds a HTTP input configuration option to support webhook/NDJSON style logging over HTTP or HTTPS via Elastic Agent inputs. Enhancements to ingest pipelines, as well as pipeline and system tests have also been made.
…elastic#11742) This commit adds BBOT v2.x NDJSON format support the integration, while simultaneously supporting the existing BBOT v1.x format. It also adds a HTTP input configuration option to support webhook/NDJSON style logging over HTTP or HTTPS via Elastic Agent inputs. Enhancements to ingest pipelines, as well as pipeline and system tests have also been made.





Proposed commit message
This commit adds BBOT v2.x NDJSON format support the integration, while simultaneously supporting the existing BBOT v1.x format.
It also adds a HTTP input configuration option to support webhook/NDJSON style logging over HTTP or HTTPS via Elastic Agent inputs.
Enhancements to ingest pipelines, as well as pipeline and system tests have also been made.
Checklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
elastic-agent testRelated issues
Screenshots
Not applicable.
See Also
https://blog.blacklanternsecurity.com/p/bbot-20-release-announcement