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

Field mapping validator #8476

Closed
wants to merge 2 commits into from

Conversation

adriansr
Copy link
Contributor

This patch adds a document validator that compares an Elasticsearch document in JSON format against a Beat fields in Yaml format.

It checks that:

  • All fields in a document have a mapping.
  • Fields types match the mapping.
  • Required fields are present.

Currently it only supports a limited subset of datatypes. Needs to be extended with more types.

// ValidateDocument takes a document from Elasticsearch in JSON format
// and the contents of a Beat's fields.yml and validated the document's
// fields against it.
func ValidateDocument(docJson []byte, fieldsYaml []byte) error {

Choose a reason for hiding this comment

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

func name will be used as validate.ValidateDocument by other packages, and that stutters; consider calling this Document
func parameter docJson should be docJSON

return seen, nil
}

func (m *Mapping) AddField(path string, field Field, required bool) error {

Choose a reason for hiding this comment

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

exported method Mapping.AddField should have comment or be unexported

@adriansr adriansr force-pushed the feature-suricata branch 4 times, most recently from 9fcdddc to b113cd5 Compare September 27, 2018 21:42
@adriansr adriansr requested a review from andrewkroh September 27, 2018 21:43
return nil
}

func recursiveFattenFields(fields interface{}, prefix Prefix, mapping *Mapping, key string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Reminds me a lot of the code in #7972

// specific language governing permissions and limitations
// under the License.

package php_fpm

Choose a reason for hiding this comment

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

don't use an underscore in package name


// +build !integration

package node_stats

Choose a reason for hiding this comment

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

don't use an underscore in package name

)
}

// OrderIntervalLogs, when given a log filename in the form [prefix]-[formattedDate]-n

Choose a reason for hiding this comment

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

comment on exported function OrderIntervalLogs should be of the form "OrderIntervalLogs ..."

var modulesDirGenerated = filepath.Clean("build/packaging/modules")
var modulesDirGeneratedOSS = filepath.Clean("build/packaging/modules-oss")
var modulesDirGeneratedXPack = filepath.Clean("build/packaging/modules-x-pack")
var modules_D_DirGeneratedXpack = filepath.Clean("build/packaging/modules.d-x-pack")

Choose a reason for hiding this comment

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

don't use underscores in Go names; var modules_D_DirGeneratedXpack should be modulesDDirGeneratedXpack

This patch adds a document validator that compares an Elasticsearch
document in JSON format against a Beat fields in Yaml format.

It checks that:
- All fields in a document have a mapping.
- Fields types match the mapping.
- Required fields are present.

Currently it only supports a limited subset of datatypes. Needs to be
extended with more types.
Learned some fixes from running it against current Beats:
- Some sub-fields only use the release tag, for documentation purposes.
- object used as synonym for group.
- type group without fields tag, treat as empty fields.
@adriansr
Copy link
Contributor Author

adriansr commented Oct 3, 2018

These commits were merged (by mistake) into master in #8313

Luckily this PR was already green and approved when that happened.

Sorry about that.

@adriansr adriansr closed this Oct 3, 2018
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.

3 participants