Conversation
Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>
Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>
Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>
Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>
…of the repo cover the code here
….hbs Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>
Changed {{#each paths as |path i|}} to {{#each paths as |path|}} per efd6 request
Added new line to end per efd6 request.
Updated ecs.version per efd6 request
Added new line to end per efd6 request
Added descriptions to names.
Removed Asset Identification and updated exported field for tychon_cve
Bruce updates
❕ Build Aborted
Expand to view the summary
Build stats
Steps errors
Expand to view the steps failures
|
|
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
|
/test |
efd6
left a comment
There was a problem hiding this comment.
This PR looks to be a revival of #6701 with a large number of additions. Ideally, I would like a fresh start based on the current main with some separation of the data streams that are being added. My suggestion would be to start with a single data stream and its associated dashboard and then add subsequent datastreams/dashboards when we have a good success with the first and we are in our stride.
|
My proposed approach would be to:
Then we can focus on a limited set of changes to get things right and then progress from there. |
|
I have pushed a copy of the code here, squashed/rebase onto main to #8141. I have also cleaned up typographic issues with the code (you can see what was done in the last three commits, but briefly: changed line endings from dos crlf to unix lf only; removed trailing whitespace on lines; and added final new lines to yaml and hbs files). Please use the state there as a starting point. |
|
@efd6 thanks for working this with us, quick question, should the versioning move to 1.0? This is already in production and going back to a .0.0.1 will not allow customers to upgrade in the future. |
|
@joeperuzzi The version of the package is independent of the version of the product that is being integrated. The tychon package here is new, so we can start at any version. When you say that it is already in production, do you mean that there are users that have this package installed external to EPR? |
@efd6 we tried to get the first version through review before deploying to customer sites but it took longer than expected so I closed that pull request for this one with all the new datasets. We have deployed this plugin to multiple sites for pilots and existing customers. We installed via air gapped registration servers as we were unable to install via normal methods. There are more datasets on the way so I’d like to see this deploy as 1.0 so we can iterate higher as we release more source data. |
|
OK. So given how I would like this to proceed, I'm happy for that to happen, but I would like to use a sub-1.0.0 version until we have everything sorted and then bump to 1. |
|
@efd6 I'm struggling a bit trying to break this code up and have it still install/function. We did not intend for this to be pieced together and therefore its making me very nervous to break this code up into pieces. This is a well-tested integration and my fear is that I'm taking production ready code and breaking it up for ease of review putting a ton of risk for defects once released. Would it be better to leave everything intact and then give you a break-down of each stream and their associated parts (dashboards, siem rules, transforms, etc..). Then you can focus the data streams one at a time without having a lot of different branches between internal testing, code review, and this pull request. |
|
Sorry, I won't open a docx. Are you saying that there is interaction between the different datastreams? I'm not entirely sure what to do here. The package does not pass when I test it locally (the branch at the PR I linked above), and I would like to ensure that our code standards are met when we add this. To do that over a 20kSLOC addition in one go is not going to work well. I think it would be much better to add a single datastream, iron out the issues in that and then allow you to take what is learned there and apply that to other datastreams. |
|
@efd6 yes visuals and rules have overlap between the datasreams. Looking at your build output it seems like the sample_event.json files are not correct in those few items, we'll remove the sample_event.json files as they are going unused and have no bearing on the functionality of the product. One error I'm seeing is "[2] parsing field value failed: field "event.outcome"'s value "fail" is not one of the allowed values (failure, success, unknown)". This dataset is handled in the pipeline, and when looking at the _dev/test/pipeline folder you'll notice the expected outcome matches failure/success/unknown. But once we remove the sample_event.json that should take care of this error.
|
Readme update
Removed old sample_event.json data
|
Removed the 4 sample_event.json files from datastreams "cve, epp, networkadapter, and stig". This should correct the build errors on data being undefined. The sample_event.json files were not being used as we felt these would clutter the readme file and this data is better defined in the TYCHON documentation. |
|
@efd6 we're going to push a new PR with the first stand-alone datastream in it, but I'm going to leave this PR open. We couldn't get it to build correctly as there are overlapping artifacts between streams so hopefully you're good with pull requests that have build errors that are not recoverable. Unfortunately with this process, it's not possible to get it to build without having all the needed artifacts from other streams. We'll have the first push later this afternoon for review and I'll update both tickets to point to each-other. |
|
I'm going to push back on leaving this PR open; there are 82 commit here making review communication more difficult. I would really like to have this simplified by starting from a base of main, with the state that is at the PR that I linked as it fixes things that will need to be fixed in the package before it can move forward.
OK, so how about we start with adding just the pipelines and tests (including system tests which are absent here). Then, when they are all in, we can add the cross-dependent layers. |
|
Closing for PR: #8212 |
Proposed commit message
Checklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
Please reach out to support@tychon.io to get a copy of TYCHON Agentless if testing is needed.