METRON-1608: Add configuration for threat.triage.field name#1055
METRON-1608: Add configuration for threat.triage.field name#1055merrimanr wants to merge 2 commits intoapache:masterfrom
Conversation
| this.pageSize = pageSize; | ||
| } | ||
|
|
||
| private String getField(String globalConfigKey, String defaultField) { |
There was a problem hiding this comment.
It seems like we have duplicated this logic here and in SearchServiceImpl. Would it make sense to extract this out?
There was a problem hiding this comment.
The latest commit extracts this to ConfigurationsUtils class in metron-common. This class is already really big so I was hesitant to add it there but I'm not sure where else it belongs.
| @@ -151,7 +152,7 @@ public List<String> getDefaultFacetFields() throws RestException { | |||
| String sourceTypeField = Constants.SENSOR_TYPE.replace('.', ':'); | |||
There was a problem hiding this comment.
Just another point of duplication. We define what the default value is here and in ElasticsearchMetaAlertDao. Could be extracted out along with the other bits.
There was a problem hiding this comment.
This one is tricky. We can't just reference ElasticsearchMetaAlertDao.SOURCE_TYPE because metron-elasticsearch is a runtime dependency in metron-rest. The only other solution I can think of is to define a second constant in Constants but I think that is confusing since it's specific to ES. I remember @justinleet working through this in another, maybe he can chime in.
|
FYI - There is also another |
|
Good feedback @nickwallen. The latest commit should satisfy your comments except the duplicated SOURCE_TYPE issue. Not sure what the best approach is so I'm inclined to leave it alone. |
|
+1 looks good. Thanks I wouldn't worry about that constant. Not worth it. |
Contributor Comments
This PR adds a configuration to the global config for the
threat.triage.scorefield name, similar to what was done in #1010 forsource.type, minus the UI changes.I also opportunistically fixed a bug where the
source.typefield name wasn't being read from the global config. Normally this would be a separate PR but the fix overlaps with what was done here so I included it. I also added a constant for thesource.type.fieldproperty.Changes
source.type.fieldandthreat.triage.field(wouldthreat.triage.score.fieldbe better?)getFieldmethod that gets the field name from the global config or returns the default field name if not foundsource.type.fieldandthreat.triage.fieldTesting
Testing this in full dev requires changing our indexing topology to use fields with '.'s rather than ':'s. To do that:
service sensor-stubs stop snortcurl -XDELETE http://node1:9200/snort_index*/var/lib/ambari-agent/cache/common-services/METRON/0.5.0/package/files/snort_index.templateby changingsource:typetosource.typeandthreat:triage:*scoretothreat.triage.*scoreservice sensor-stubs start snortBefore this PR there were a couple problems. First the metaalert had the
source:typefield even though we've switched to "source.type" in the global config. Second, thethreat.triage.scorefield in the metaalert would be 0 because the wrong threat triage field name is used to get scores from alerts. With this PR the metalaert should correctly have thesource.typefield and thethreat.triage.scorefield should be > 0 (assuming the snort alert has a threat triage score):Pull Request Checklist
Thank you for submitting a contribution to Apache Metron.
Please refer to our Development Guidelines for the complete guide to follow for contributions.
Please refer also to our Build Verification Guidelines for complete smoke testing guides.
In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:
For all changes:
For code changes:
Have you included steps to reproduce the behavior or problem that is being changed or addressed?
Have you included steps or a guide to how the change may be verified and tested manually?
Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
Have you written or updated unit tests and or integration tests to verify your changes?
If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
For documentation related changes:
Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via
site-book/target/site/index.html:Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
It is also recommended that travis-ci is set up for your personal repository such that your branches are built there before submitting a pull request.