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

[Filebeat] NetFlow input support for custom field definitions #10945

Merged
merged 11 commits into from
Mar 6, 2019

Conversation

adriansr
Copy link
Contributor

This PR adds support for loading custom (enterprise-specific) fields to the Filebeat NetFlow input.
These fields can extend and/or override fields in NetFlow V9 and IPFIX.

For compatibility, the feature uses the same field definition YAML format as Logstash's netflow codec plugin.

A new configuration option custom_definitions consists of a list of paths to definition files.

@adriansr adriansr added enhancement review Filebeat Filebeat needs_backport PR is waiting to be backported to other branches. SecOps labels Feb 26, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/secops

return nil
}

var emptyDefinitionErr = errors.New("custom fields definition is empty")

Choose a reason for hiding this comment

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

error var emptyDefinitionErr should have name of the form errFoo

@@ -92,7 +94,15 @@ func (d DecoderV9) ReadFieldDefinition(buf *bytes.Buffer) (field fields.Key, len
return field, length, nil
}

func (d DecoderV9) GetFields() fields.FieldDict {

Choose a reason for hiding this comment

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

exported method DecoderV9.GetFields should have comment or be unexported

@@ -20,12 +20,20 @@ type Field struct {

type FieldDict map[Key]*Field

func RegisterFields(dict FieldDict) error {
func RegisterGlobalFields(dict FieldDict) error {

Choose a reason for hiding this comment

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

exported function RegisterGlobalFields should have comment or be unexported

@@ -6,7 +6,7 @@ package fields

import "fmt"

var Fields = FieldDict{}
var GlobalFields = FieldDict{}

Choose a reason for hiding this comment

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

exported var GlobalFields should have comment or be unexported

Added parsing for Logstash's Custom fields definitions. They allow to
extend the NetFlow/IPFIX collector supported fields.
Extend NetFlow collector library to configure custom fields
for NetFlow V9 / IPFIX.
Added a new configuration option, `custom_definitions`, allows a list of
paths to YAML field definitions in Logstash codec-netflow format.
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.

Can you add the config options to to docs.

}

// LoadFieldDefinitionsFromFile takes the path to a YAML file in Logstash
// Netflow or IPFIX custom fields format and converts it to a FieldDict.
Copy link
Member

Choose a reason for hiding this comment

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

We'll need a description of the config format in the Filebeat docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

Two nits and a minor request on adding a test.

👍


Where `id` is the numeric field ID.

The IPFIX format similar, but grouped by Private Enterprise NUmber (PEN):
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I assume "Number" should be the capitalization here? ;-)

t.FailNow()
}

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps have a test demonstrating the direction of the overwrite, when a key is present in multiple sources? None of the fields in the test above conflict.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a conflict with Key{3, 4}, which is overwritten by c. Is that what you mean?

@@ -148,7 +154,7 @@ func readDatTests(t testing.TB) *DatTests {
return &tests
}

func getFlowsFromDat(t testing.TB, name string, datFiles ...string) TestResult {
func getFlowsFromDat(t testing.TB, name string, testData DatTest) TestResult {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This mixed naming in var/funcs named dat vs data is confusing. Both are right, but I think if you pick one or the other across the board, things will be more clear. I think "data" better describes it.

@adriansr adriansr merged commit cd49078 into elastic:master Mar 6, 2019
adriansr added a commit to adriansr/beats that referenced this pull request Mar 13, 2019
…c#10945)

This PR adds support for loading custom (enterprise-specific) fields to the 
Filebeat NetFlow input. These fields can extend and/or override fields in
NetFlow V9 and IPFIX.

For compatibility, the feature uses the same field definition YAML format
as Logstash's netflow codec plugin.

A new configuration option custom_definitions consists of a list of paths
to definition files.

(cherry picked from commit cd49078)
@adriansr adriansr added v7.0.0 and removed needs_backport PR is waiting to be backported to other branches. labels Mar 13, 2019
adriansr added a commit that referenced this pull request Mar 13, 2019
…m field definitions (#11223)

[Filebeat] NetFlow input support for custom field definitions (#10945)

This PR adds support for loading custom (enterprise-specific) fields to the 
Filebeat NetFlow input. These fields can extend and/or override fields in
NetFlow V9 and IPFIX.

For compatibility, the feature uses the same field definition YAML format
as Logstash's netflow codec plugin.

A new configuration option custom_definitions consists of a list of paths
to definition files.

(cherry picked from commit cd49078)
adriansr added a commit to adriansr/beats that referenced this pull request Mar 13, 2019
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.

5 participants