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] Update CoreDNS pipelines for ECS DNS #13505

Merged
merged 3 commits into from
Sep 11, 2019

Conversation

andrewkroh
Copy link
Member

This sets the ECS DNS fields. It does not remove the coredns.* fields to avoid introducing
a breaking change.

Relates #13320

@andrewkroh andrewkroh requested a review from a team as a code owner September 5, 2019 05:05
@andrewkroh andrewkroh force-pushed the feature/coredns-ecs-conversion branch from 42b0544 to e3c6446 Compare September 5, 2019 05:05
This sets the ECS DNS fields. It does not remove the coredns.* fields to avoid introducing
a breaking change.

Relates elastic#13320
@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem

@andrewkroh andrewkroh mentioned this pull request Sep 5, 2019
6 tasks
@andrewkroh andrewkroh changed the title Update pipelines for ECS DNS [Filebeat] Update CoreDNS pipelines for ECS DNS Sep 5, 2019
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Should we create an issue to remember removing the duplicated coredns fields on 8.0?

@andrewkroh
Copy link
Member Author

There one unrelated failure on Jenkins/Linux/libbeat:

command [go test -race -tags integration -cover -coverprofile /tmp/gotestcover-240397836 github.com/elastic/beats/libbeat/autodiscover/providers/docker]: exit status 1
--- FAIL: TestDockerStart (13.81s)
docker_integration_test.go:116: Timeout waiting for provider events
FAIL
coverage: 76.7% of statements
FAIL github.com/elastic/beats/libbeat/autodiscover/providers/docker 14.564s

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 small things and we're good to go!

- set:
if: ctx.dns?.question?.name != null
field: coredns.query.name
value: '{{dns.question.name}}'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also add the registered_domain processor, to set dns.question.registered_domain?

Copy link
Member Author

Choose a reason for hiding this comment

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

It will take some refactoring because all of the processing is happening in Ingest Node with this pipeline. To use the Beat's registered_domain I'll need to move some of the dissecting to Beats.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, with all the YAML, I hadn't actually noticed that this was an ingest pipeline :-)

Let's call this out of scope for now, then?

],
"dns.id": "6966",
"dns.question.class": "IN",
"dns.question.name": "httpbin.org.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you remove the trailing dot from domain lookups?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call.

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.

LGTM, thanks!

@andrewkroh andrewkroh merged commit 8f2216e into elastic:master Sep 11, 2019
@urso urso added the v7.5.0 label Oct 22, 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