-
Notifications
You must be signed in to change notification settings - Fork 3
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
Improve arkime config files & containers #41
Conversation
awick
commented
May 1, 2023
- use BRIDGE mode for capture container
- give capture container NET_ADMIN cap so ip and /dev/net stuff works
- reduce viewer nodes min:2 max:4
- organize capture/viewer config files
- new capture default rules file
- download capture oui/rir geo files
- switch capture to afpacket mode
* use BRIDGE mode for capture container * give capture container NET_ADMIN cap so ip and /dev/net stuff works * reduce viewer nodes min:2 max:4 * organize capture/viewer config files * new capture default rules file * download capture oui/rir geo files * switch capture to afpacket mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Before I approve though, would you mind explaining what tests you performed to ensure the changes operate as expected?
@@ -0,0 +1,21 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, will we want to make the details here configurable via the CLI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think eventually we will want folks to be able to add their own rules, and maybe disable these.
The rulesFile option can take multiple files, my hack solution would be to allow the cli to upload the rules files to s3 and then have the container download them on container start up and check every X minutes for change. Capture can auto reload a changed rules file and doesn't need to restart.
we could also make the rulesFile option take a url and have capture download the new rules instead of something else, but that would take more work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it - thanks for the context!
Sure, i deployed it into my cluster and made sure it continued to worked. This involved:
|
Are there a set of tests you want me to run each time? Do we have some automated tests we can run in github actions, or do they require an AWS account? |
I don't necessarily have a specific set of tests I want run, per se, but I think being proactive in explaining the test plan for a change helps everyone involved, and all future versions of those people, to better understand the change and its risks. It's something I always ask on a change request because I've been burned in the past by not asking, and what I'm looking for is some set of actions that provide a plausible coverage of the change and whose thoroughness fits where the project is in its lifecycle. Also, as a general practice, I find it gets people thinking about testing earlier in a project if they know I'll ask about how they testing things before approving a change. Not saying anyone currently working on the project needs that reinforcement; it's just a rule I follow on every project I work on.
We have the 110+ unit tests for the Python side of things which we could (and should) easily set up automated testing for in GitHub. Unfortunately, much of the "interesting" behavior requires an AWS account to provide a reasonable test plan for. Not sure what our options are on that front given the open-source nature of the project (who pays the bill, access management, etc). |
If/when we start having additional contributors to the project and/or seeing adoption, we'll want to revisit this topic and come up with more concrete test standards. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ship it!