Skip to content

[CoreDNS] Add CoreDNS package#4064

Merged
andrewkroh merged 14 commits intoelastic:mainfrom
legoguy1000:coredns-logs
Oct 9, 2022
Merged

[CoreDNS] Add CoreDNS package#4064
andrewkroh merged 14 commits intoelastic:mainfrom
legoguy1000:coredns-logs

Conversation

@legoguy1000
Copy link
Copy Markdown
Contributor

What does this PR do?

Add CoreDNS package

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

  • [ ]

How to test this PR locally

Related issues

Screenshots

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Aug 25, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-10-08T14:21:25.698+0000

  • Duration: 15 min 20 sec

Test stats 🧪

Test Results
Failed 0
Passed 13
Skipped 0
Total 13

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

@legoguy1000 legoguy1000 marked this pull request as ready for review August 28, 2022 22:07
@elasticmachine
Copy link
Copy Markdown

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

@jamiehynds jamiehynds requested a review from a team September 20, 2022 09:54
Copy link
Copy Markdown
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.

Great to see CoreDNS support coming to Agent.

I think this package would benefit from a system test the runs the real CoreDNS in a container, and then sends in some queries from another container using dig to simulate requests and possibly some errors. I used a setup like this in the hashicorp_vault package and it makes verifying compatibility with new versions easier.

This would also setup the package to have a metrics data_stream added that uses the prometheus input to collect data from https://coredns.io/plugins/metrics/. (Also did this in hashicorp_vault.)

@legoguy1000
Copy link
Copy Markdown
Contributor Author

Great to see CoreDNS support coming to Agent.

I think this package would benefit from a system test the runs the real CoreDNS in a container, and then sends in some queries from another container using dig to simulate requests and possibly some errors. I used a setup like this in the hashicorp_vault package and it makes verifying compatibility with new versions easier.

This would also setup the package to have a metrics data_stream added that uses the prometheus input to collect data from https://coredns.io/plugins/metrics/. (Also did this in hashicorp_vault.)

I was going to do this but couldn't get CoreDNS to log to a file within a container so with the current inputs wouldn't really allow for reading from the container STDOUT unless I'm missing something

@andrewkroh
Copy link
Copy Markdown
Member

andrewkroh commented Sep 29, 2022

I would tee the stdout from the CoreDNS process to a file that the system tests can consume. For example

docker-entrypoint.sh server -dev 2>&1 | tee /service_logs/log.json

@legoguy1000
Copy link
Copy Markdown
Contributor Author

I would tee the stdout from the CoreDNS process to a file that the system tests can consume. For example

docker-entrypoint.sh server -dev 2>&1 | tee /service_logs/log.json

entrypoint: /coredns -conf /config/Corefile 2>&1 | tee /tmp/coredns.log results in 2022/10/04 02:17:01 extra command line arguments: [2>&1 | tee /tmp/coredns.log] so still need to find away around it in order to use the coredns image instead of just the alpine.

@andrewkroh
Copy link
Copy Markdown
Member

The coredns image is bult FROM scratch and only holds the coredns binary and ca-certs. How about as a workaround building an image based on something small like debian-slim? coredns is only single binary so the Dockerfile should be fairly simple.

@legoguy1000
Copy link
Copy Markdown
Contributor Author

The coredns image is bult FROM scratch and only holds the coredns binary and ca-certs. How about as a workaround building an image based on something small like debian-slim? coredns is only single binary so the Dockerfile should be fairly simple.

That worked. Just need to figure out the errors now with the agent.

@legoguy1000
Copy link
Copy Markdown
Contributor Author

@andrewkroh fixed tests. ready for review

Copy link
Copy Markdown
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.

LGTM. Can you please include a screenshot of the dashboard in the package.

Copy link
Copy Markdown
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.

LGTM 🚢

@andrewkroh
Copy link
Copy Markdown
Member

/test

@elasticmachine
Copy link
Copy Markdown

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@elasticmachine
Copy link
Copy Markdown

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 100.0% (2/2) 💚 2.726
Classes 100.0% (2/2) 💚 2.726
Methods 95.455% (21/22) 👍 5.583
Lines 85.492% (165/193) 👎 -6.159
Conditionals 100.0% (0/0) 💚

@andrewkroh andrewkroh merged commit fd89984 into elastic:main Oct 9, 2022
@legoguy1000 legoguy1000 deleted the coredns-logs branch October 9, 2022 14:44
@ruflin ruflin mentioned this pull request Oct 11, 2022
4 tasks
@andrewkroh andrewkroh added Integration:coredns CoreDNS New Integration Issue or pull request for creating a new integration package. labels Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Integration:coredns CoreDNS New Integration Issue or pull request for creating a new integration package.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants