network_traffic: expose configuration options to users #3157

efd6 · 2022-04-21T09:35:34Z

What does this PR do?

This exposes the majority of available configuration options for the network traffic data streams and removes the command line grep process name rewrite configuration.

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

Compare with packetbeat.reference.yml for options for each data stream.

How to test this PR locally

elastic-package test in packages/network_traffic.

Related issues

Screenshots

Single example:

elasticmachine · 2022-04-21T09:43:45Z

💚 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

Start Time: 2022-05-11T07:19:12.284+0000
Duration: 71 min 58 sec

Test stats 🧪

Test	Results
Failed	0
Passed	264
Skipped	0
Total	264

🤖 GitHub comments

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

/test : Re-trigger the build.

P1llus

This is my initial review that was requested, covering mostly just manifest and hbs files.

I think the discussion we had around what should be show_user true and what should be false
Should simply drill down to this:

If the option is usually never used, and usually requires complex configuration, it should not be shown to user (which means it is moved under advanced options).
If we end up with a lot of options set to true, try to minimize them even more with sane defaults, so that the initial count of available options is low.

Currently it seems to follow this principle, so it's not a comment against the current state of the config, but rather the thought around what should be true and what should be false.

P1llus · 2022-04-26T07:07:12Z

packages/network_traffic/data_stream/amqp/agent/stream/amqp.yml.hbs

+{{#if transaction_timeout}}
+transaction_timeout: {{transaction_timeout}}
+{{/if}}
+{{#if index}}


Please remove index from all the configs, and change it out with dataset.datastream
ref here: https://github.com/elastic/integrations/blob/main/packages/tcp/data_stream/generic/agent/stream/tcp.yml.hbs#L1

I have removed the data_stream.dataset since it was causing the tests to fail.

Change here.

packages/network_traffic/data_stream/amqp/agent/stream/amqp.yml.hbs

P1llus · 2022-04-26T07:08:33Z

packages/network_traffic/data_stream/amqp/agent/stream/amqp.yml.hbs

-    {{#each processes}}
-    - cmdline_grep: {{this}}
+    {{#each processes as |process|}}
+    - cmdline_grep: {{process}}
    {{/each}}
 {{/if}}
 {{#if interface}}


Please add support for custom processors, similar to here:
https://github.com/elastic/integrations/blob/main/packages/tcp/data_stream/generic/agent/stream/tcp.yml.hbs#L38

P1llus · 2022-04-26T07:10:02Z

packages/network_traffic/data_stream/amqp/manifest.yml

@@ -13,7 +13,86 @@ streams:
        required: true


If it does not support multi, it might be wise that we change this to type yaml, instead of text or integer, to allow for array support?

Will that get around the kibana issue?

When using mutli: true with type: text I can still pass numbers through the policy (tested this with tags). One option could be to change the type to text and forego the number validation until this is fixed. And speaking of fixes, I cannot find an open issue in elastic/kibana for supporting multi + integer, can you please open one, @efd6?

elastic/kibana#131360

I have tried multi: true with type: text and I am unable to get this to work; the tests fail with {"statusCode":400,"error":"Bad Request","message":"Package policy is invalid: inputs.packet.streams.network_traffic.http.vars.port: Invalid format"}.

With tags the handlebar template uses a for loop to construct the value. Maybe try that format if you haven't already.

Yes that was one of the many already tried.

Using multi: true with type: text and for loop in the Handlebar template is working for me on 7.17.0.

Generated policy:

- id: packet-network-9c3f6353-0533-4ad9-828f-fd99cf93f3f1 name: network_traffic-1 revision: 1 type: packet use_output: default meta: package: name: network_traffic version: 0.10.0 data_stream: namespace: default streams: - id: packet-network_traffic.dhcpv4-9c3f6353-0533-4ad9-828f-fd99cf93f3f1 type: dhcpv4 data_stream: dataset: network_traffic.dhcpv4 type: logs ports: - 67 - 68

diff --git a/packages/network_traffic/data_stream/dhcpv4/agent/stream/dhcpv4.yml.hbs b/packages/network_traffic/data_stream/dhcpv4/agent/stream/dhcpv4.yml.hbs index 3856fc4d9..dba4a3a7b 100644 --- a/packages/network_traffic/data_stream/dhcpv4/agent/stream/dhcpv4.yml.hbs +++ b/packages/network_traffic/data_stream/dhcpv4/agent/stream/dhcpv4.yml.hbs @@ -1,5 +1,12 @@ type: dhcpv4 -ports: [{{port}}] + +{{#if port}} +ports: +{{#each port as |p|}} + - {{p}} +{{/each}} +{{/if}} + {{#if keep_null}} keep_null: {{keep_null}} {{/if}} diff --git a/packages/network_traffic/data_stream/dhcpv4/manifest.yml b/packages/network_traffic/data_stream/dhcpv4/manifest.yml index 8e5ff7b20..646b0cc19 100644 --- a/packages/network_traffic/data_stream/dhcpv4/manifest.yml +++ b/packages/network_traffic/data_stream/dhcpv4/manifest.yml @@ -5,14 +5,14 @@ streams: - input: packet vars: - name: port - type: integer + type: text # currently the Kibana UI doesn't support multi inputs # that are numeric, you get "Error: r.toLowerCase is not a function" - # multi: true - title: Port + multi: true + title: Ports required: true show_user: true - default: 67 # default: [67, 68] + default: [67, 68] - name: keep_null type: bool title: Keep Null

elasticmachine · 2022-04-28T06:36:48Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`0/0`)	💚
Files	100.0% (`0/0`)	💚 3.659
Classes	100.0% (`0/0`)	💚 3.659
Methods	71.875% (`46/64`)	👎 -17.009
Lines	100.0% (`0/0`)	💚 9.897
Conditionals	100.0% (`0/0`)	💚

packages/network_traffic/data_stream/amqp/agent/stream/amqp.yml.hbs

elasticmachine · 2022-04-28T22:21:46Z

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

andrewkroh · 2022-05-02T13:02:27Z

packages/network_traffic/data_stream/amqp/manifest.yml

@@ -13,7 +13,86 @@ streams:
        required: true


When using mutli: true with type: text I can still pass numbers through the policy (tested this with tags). One option could be to change the type to text and forego the number validation until this is fixed. And speaking of fixes, I cannot find an open issue in elastic/kibana for supporting multi + integer, can you please open one, @efd6?

packages/network_traffic/data_stream/tls/manifest.yml

packages/network_traffic/data_stream/pgsql/manifest.yml

andrewkroh

The current state looks very good. I would like to get the port issue resolved since that will make this much more usable. Can you try repeating my results with the provided diff.

andrewkroh · 2022-05-10T19:28:19Z

packages/network_traffic/data_stream/amqp/manifest.yml

@@ -13,7 +13,86 @@ streams:
        required: true


Using multi: true with type: text and for loop in the Handlebar template is working for me on 7.17.0.

Generated policy:

- id: packet-network-9c3f6353-0533-4ad9-828f-fd99cf93f3f1 name: network_traffic-1 revision: 1 type: packet use_output: default meta: package: name: network_traffic version: 0.10.0 data_stream: namespace: default streams: - id: packet-network_traffic.dhcpv4-9c3f6353-0533-4ad9-828f-fd99cf93f3f1 type: dhcpv4 data_stream: dataset: network_traffic.dhcpv4 type: logs ports: - 67 - 68

diff --git a/packages/network_traffic/data_stream/dhcpv4/agent/stream/dhcpv4.yml.hbs b/packages/network_traffic/data_stream/dhcpv4/agent/stream/dhcpv4.yml.hbs index 3856fc4d9..dba4a3a7b 100644 --- a/packages/network_traffic/data_stream/dhcpv4/agent/stream/dhcpv4.yml.hbs +++ b/packages/network_traffic/data_stream/dhcpv4/agent/stream/dhcpv4.yml.hbs @@ -1,5 +1,12 @@ type: dhcpv4 -ports: [{{port}}] + +{{#if port}} +ports: +{{#each port as |p|}} + - {{p}} +{{/each}} +{{/if}} + {{#if keep_null}} keep_null: {{keep_null}} {{/if}} diff --git a/packages/network_traffic/data_stream/dhcpv4/manifest.yml b/packages/network_traffic/data_stream/dhcpv4/manifest.yml index 8e5ff7b20..646b0cc19 100644 --- a/packages/network_traffic/data_stream/dhcpv4/manifest.yml +++ b/packages/network_traffic/data_stream/dhcpv4/manifest.yml @@ -5,14 +5,14 @@ streams: - input: packet vars: - name: port - type: integer + type: text # currently the Kibana UI doesn't support multi inputs # that are numeric, you get "Error: r.toLowerCase is not a function" - # multi: true - title: Port + multi: true + title: Ports required: true show_user: true - default: 67 # default: [67, 68] + default: [67, 68] - name: keep_null type: bool title: Keep Null

efd6 · 2022-05-10T23:46:36Z

I'm convinced I tried this; obviously not.

efd6 · 2022-05-11T01:40:48Z

Locally, this works for all of the data streams except http and redis. I have done a fair amount of surgical dissection, but have not been able to find the difference between it and the multiport data streams that do work. I'm pushing to see if this is just a local issue; nope also fails, but differently — it appears 8.2 validates and fails out early while 7.17 forges ahead fruitlessly.

The error that is reported is what I was getting previously:

{
   "statusCode":400,
   "error":"Bad Request",
   "message":"Package policy is invalid: inputs.packet.streams.network_traffic.http.vars.port: Invalid format"
}

Because it's being rejected, I'm not sure how to check what it thinks it is being told.

This was resolved by monkeying in the running kibana container with

sed -i "s/'Invalid format'/'Invalid format: '+String(parsedValue)+' ('+Object.prototype.toString.call(parsedValue)+')'/" x-pack/plugins/fleet/common/services/validate_package_policy.js

(there is no vi in the container)

and restarting the container to see the value that it's being given. This showed that the values were scalars. These were coming from the test configs.

[git-generate] go run github.com/efd6/generatentconf@4016d320082df0f9ecba567a6115a2e4966d1b98 -root packages/network_traffic

efd6 · 2022-05-11T08:33:21Z

🎉

efd6 force-pushed the 2860-network-traffic branch from 220650d to 54e96b0 Compare April 21, 2022 11:39

efd6 self-assigned this Apr 21, 2022

efd6 force-pushed the 2860-network-traffic branch 7 times, most recently from 51157bb to 35c037f Compare April 26, 2022 06:57

P1llus reviewed Apr 26, 2022

View reviewed changes

efd6 force-pushed the 2860-network-traffic branch from 35c037f to dac901b Compare April 28, 2022 01:32

efd6 requested a review from P1llus April 28, 2022 01:37

efd6 force-pushed the 2860-network-traffic branch from dac901b to 14dda19 Compare April 28, 2022 04:43

efd6 added 2 commits April 28, 2022 14:15

make file names consistent where possible

3b2d504

update changelog and manifest

1bf369c

efd6 force-pushed the 2860-network-traffic branch from 14dda19 to b5a21ed Compare April 28, 2022 04:46

efd6 force-pushed the 2860-network-traffic branch 2 times, most recently from 3ad6265 to 3ee3614 Compare April 28, 2022 12:41

efd6 added enhancement New feature or request Team:Security-External Integrations Integration:network_traffic Network Packet Capture labels Apr 28, 2022

efd6 changed the title ~~initial sketch for config changes~~ network_traffic: expose configuration options to users Apr 28, 2022

efd6 commented Apr 28, 2022

View reviewed changes

packages/network_traffic/data_stream/amqp/agent/stream/amqp.yml.hbs Outdated Show resolved Hide resolved

efd6 marked this pull request as ready for review April 28, 2022 22:21

efd6 requested a review from a team as a code owner April 28, 2022 22:21

andrewkroh reviewed May 2, 2022

View reviewed changes

attach comment of preferred default to correct yaml node

d72435e

efd6 force-pushed the 2860-network-traffic branch 3 times, most recently from b14f81e to 3670582 Compare May 6, 2022 08:36

andrewkroh reviewed May 10, 2022

View reviewed changes

map ports as multi text

29514bd

efd6 force-pushed the 2860-network-traffic branch from 3670582 to a4645c3 Compare May 11, 2022 01:25

efd6 added 2 commits May 11, 2022 16:39

prepare tests for multi ports

d168b9a

generate config files

875c0f8

[git-generate] go run github.com/efd6/generatentconf@4016d320082df0f9ecba567a6115a2e4966d1b98 -root packages/network_traffic

efd6 force-pushed the 2860-network-traffic branch from 212952f to 875c0f8 Compare May 11, 2022 07:18

efd6 mentioned this pull request May 11, 2022

network_traffic using invalid ECS values #3329

Closed

andrewkroh approved these changes May 11, 2022

View reviewed changes

efd6 merged commit 0d3be38 into elastic:main May 11, 2022

efd6 mentioned this pull request May 18, 2022

network_traffic: add docs for configuration options #3371

Merged

5 tasks

andrewkroh mentioned this pull request May 22, 2022

[production] Promote many packages from snapshot elastic/package-storage#4449

Closed

andrewkroh mentioned this pull request May 31, 2022

[production] Promote many packages from snapshot elastic/package-storage#4504

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

network_traffic: expose configuration options to users #3157

network_traffic: expose configuration options to users #3157

efd6 commented Apr 21, 2022 •

edited

Loading

elasticmachine commented Apr 21, 2022 •

edited

Loading

Build stats

Test stats 🧪

P1llus left a comment

P1llus Apr 26, 2022

efd6 Apr 28, 2022

P1llus Apr 26, 2022

P1llus Apr 26, 2022 •

edited

Loading

efd6 Apr 28, 2022

andrewkroh May 2, 2022

efd6 May 3, 2022

efd6 May 3, 2022

andrewkroh May 3, 2022 •

edited

Loading

efd6 May 3, 2022

andrewkroh May 10, 2022

elasticmachine commented Apr 28, 2022 •

edited

Loading

elasticmachine commented Apr 28, 2022

andrewkroh May 2, 2022

andrewkroh left a comment

andrewkroh May 10, 2022

efd6 commented May 10, 2022

efd6 commented May 11, 2022 •

edited

Loading

efd6 commented May 11, 2022

network_traffic: expose configuration options to users #3157

network_traffic: expose configuration options to users #3157

Conversation

efd6 commented Apr 21, 2022 • edited Loading

What does this PR do?

Checklist

Author's Checklist

How to test this PR locally

Related issues

Screenshots

elasticmachine commented Apr 21, 2022 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

P1llus 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

P1llus Apr 26, 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

andrewkroh May 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Apr 28, 2022 • edited Loading

🌐 Coverage report

elasticmachine commented Apr 28, 2022

Choose a reason for hiding this comment

andrewkroh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

efd6 commented May 10, 2022

efd6 commented May 11, 2022 • edited Loading

efd6 commented May 11, 2022

efd6 commented Apr 21, 2022 •

edited

Loading

elasticmachine commented Apr 21, 2022 •

edited

Loading

P1llus Apr 26, 2022 •

edited

Loading

andrewkroh May 3, 2022 •

edited

Loading

elasticmachine commented Apr 28, 2022 •

edited

Loading

efd6 commented May 11, 2022 •

edited

Loading