SeisoLLC / zeek-kafka

A Zeek log writer plugin that publishes to Kafka.
Apache License 2.0
45 stars 16 forks source link

Modernization to Zeek 4+ #23

Closed ckreibich closed 3 years ago

ckreibich commented 3 years ago

*Thank you for submitting a contribution to the zeek-kafka writer plugin.

In order to streamline the review process, we ask you follow these guidelines and complete the following template.*

Summary of the contribution

Build and install with new Zeek versions (4.0+).

Testing

The btests pass. If you're interested in package-level CI via zkg, take a look at this branch, which adds a build matrix for PRs and pushes, and a nightly build. (This is using a Github Action we now provide, so pretty new stuff.) It passes.

Checklist

ottobackwards commented 3 years ago

Thank you for the contribution, I think that we would want to update the e2e tests to run with 4.x as well.

ckreibich commented 3 years ago

Yeah that makes sense ... I didn't really dive into your test setup, tbh, but now skimmed it. Cool to see the docker-compose to actually test the export. I'll see if I can get it to work.

JonZeolla commented 3 years ago

For testing various Zeek 3.x versions you just need to change this one place. Depending on how much changed with Zeek 4 there may be some more work to do

ckreibich commented 3 years ago

Thanks Jon ... take a look at the additional commit I pushed. This bumps to 4.0.1. With Zeek 4+, zkg is included in the distribution, so I've reduced the pip3 stuff to just installing the dependencies. I'm not sure about the purpose of requirements-to-freeze.txt, but since it only contained zkg, I removed it.

ottobackwards commented 3 years ago

I ran end to end on this, and everything looks great. I'm +1 on this. My only question is if we are dropping 3.x support should we do a last 3.x release / tag FIRST and then release this? @JonZeolla

JonZeolla commented 3 years ago

Yeah I have something in mind for release management I'll throw up a PR soon. After we get that in I agree, we can merge this in. The make requirements change from this PR breaks the way I intended it use to but I was going to replace it with a Pipfile.lock soon anyway so I won't block on that