[pr] feedback from @jacknagz & @austinbyers

stream_alert/alert_processor/helpers.py

@@ -34,7 +34,7 @@ def validate_alert(alert):
     metadata_keys = {'log', 'rule_name', 'rule_description', 'type', 'source', 'outputs'}
     if not set(alert['metadata'].keys()) == metadata_keys:
         LOGGER.error('The value of the \'metadata\' key must be a map (dict) '
-                     'that contains the following keys: %s',
+                     'that contains only the following keys: %s',
                      ', '.join('\'{}\''.format(key) for key in metadata_keys))
         return False
 
@@ -44,7 +44,7 @@ def validate_alert(alert):
             if not (isinstance(alert['metadata'][key], dict) and
                     set(alert['metadata'][key].keys()) == {'service', 'entity'}):
                 LOGGER.error('The value of the \'source\' key must be a map (dict) that '
-                             'contains \'service\' and \'entity\' keys.')
+                             'contains only \'service\' and \'entity\' keys.')
                 valid = False
                 continue
 
@@ -53,7 +53,6 @@ def validate_alert(alert):
                     LOGGER.error('The value of the \'%s\' key within \'%s\' must be '
                                  'a string (str).', entry, key)
                     valid = False
-                    continue
 
         elif key == 'outputs':
             if not (isinstance(alert['metadata'][key], list) and
@@ -69,13 +68,11 @@ def validate_alert(alert):
                     LOGGER.error('The value of each entry in the \'outputs\' list '
                                  'must be a string (str).')
                     valid = False
-                    continue
 
         elif not isinstance(alert['metadata'][key], (str, unicode)):
             LOGGER.error('The value of the \'%s\' key must be a string (str), not %s',
                          key, type(alert['metadata'][key]))
             valid = False
-            continue
 
     return valid
 
@@ -91,8 +88,8 @@ def _validate_root(alert):
     """
     if not (isinstance(alert, dict) and
             set(alert.keys()) == {'record', 'metadata'}):
-        LOGGER.error('The alert must be a map (dict) that contains \'record\' '
-                     'and \'metadata\' keys.')
+        LOGGER.error('The alert must be a map (dict) that contains only '
+                     '\'record\' and \'metadata\' keys.')
         return False
 
     if not (isinstance(alert['record'], dict) and

stream_alert/alert_processor/main.py

@@ -52,7 +52,7 @@ def handler(event, context):
     function_name = context.function_name
 
     # Return the current list of statuses back to the caller
-    return [status for status in run(event, region, function_name, config)]
+    return list(status for status in run(event, region, function_name, config))
 
 def run(alert, region, function_name, config):
     """Send an Alert to its described outputs.
@@ -76,12 +76,12 @@ def run(alert, region, function_name, config):
                 }
             }
 
-        region [string]: the AWS region being used
-        function_name [string]: the name of the lambda function
-        config [dict]: the loaded configuration for outputs from conf/outputs.json
+        region [string]: The AWS region of the currently executing Lambda function
+        function_name [string]: The name of the lambda function
+        config [dict]: The loaded configuration for outputs from conf/outputs.json
 
     Returns:
-        [generator] yields back dispatch status and name of the output to the handler
+        [generator] Yields back dispatch status and name of the output to the handler
     """
     if not validate_alert(alert):
         LOGGER.error('Invalid alert:\n%s', json.dumps(alert, indent=2))

test/unit/stream_alert_alert_processor/test_helpers.py

@@ -13,75 +13,113 @@
 See the License for the specific language governing permissions and
 limitations under the License.
 '''
-from copy import deepcopy
-
-from nose.tools import assert_true, assert_false, assert_is_instance
+from nose.tools import assert_true, assert_false
 
 from stream_alert.alert_processor.helpers import (
     validate_alert
 )
 
 from unit.stream_alert_alert_processor.helpers import _get_alert
 
-def test_alert_validate_root():
-    """Alert Structure Validation"""
-    # Default valid alert to be copied/modified
+def test_valid_alert():
+    """Alert Processor Input Validation - Valid Alert Structure"""
+    # Default valid alert to test
     valid_alert = _get_alert()
 
     # Test with a valid alert structure
     assert_true(validate_alert(valid_alert))
 
-    # Root key validation
-    invalid_root_keys = deepcopy(valid_alert)
+
+def test_root_keys():
+    """Alert Processor Input Validation - Invalid Root Keys"""
+    # Default valid alert to be modified
+    invalid_root_keys = _get_alert()
+
+    # Remove 'metadata' key to break root key validation
     invalid_root_keys.pop('metadata')
 
     # Test with invalid root keys
     assert_false(validate_alert(invalid_root_keys))
 
-    # Root value validation
-    invalid_root_values = deepcopy(valid_alert)
+
+def test_metadata_value():
+    """Alert Processor Input Validation - Invalid Root Metadata Value"""
+    # Default valid alert to be modified
+    invalid_root_values = _get_alert()
+
+    # Make the 'metadata' key's value a list to break root value validation
     invalid_root_values['metadata'] = ['value']
 
     # Test with invalid root values
     assert_false(validate_alert(invalid_root_values))
 
-    # metadata key validation
-    invalid_metadata_keys = deepcopy(valid_alert)
+
+def test_metadata_keys():
+    """Alert Processor Input Validation - Metadata Keys Missing"""
+    # Default valid alert to be modified
+    invalid_metadata_keys = _get_alert()
+
+    # Alter 'metadata' keys to break validation (not all required keys)
     invalid_metadata_keys['metadata'] = {'log': 'osquery'}
 
     # Test with invalid metadata keys
     assert_false(validate_alert(invalid_metadata_keys))
 
+
+def test_metadata_source_keys():
+    """Alert Processor Input Validation - Source Keys Missing"""
+    # Default valid alert to be modified
+    invalid_metadata_source = _get_alert()
+
     # metadata > source key validation
-    invalid_metadata_source_01 = deepcopy(valid_alert)
-    invalid_metadata_source_01['metadata']['source'] = {'service': 'kinesis'}
+    invalid_metadata_source['metadata']['source'] = {'service': 'kinesis'}
 
     # Test with invalid metadata source keys
-    assert_false(validate_alert(invalid_metadata_source_01))
+    assert_false(validate_alert(invalid_metadata_source))
+
+
+def test_metadata_source_value():
+    """Alert Processor Input Validation - Source Entity Value"""
+    # Default valid alert to be modified
+    invalid_metadata_source = _get_alert()
 
     # metadata > source value validation
-    invalid_metadata_source_02 = deepcopy(valid_alert)
-    invalid_metadata_source_02['metadata']['source']['entity'] = 100
+    invalid_metadata_source['metadata']['source']['entity'] = 100
 
     # Test with invalid metadata source values
-    assert_false(validate_alert(invalid_metadata_source_02))
+    assert_false(validate_alert(invalid_metadata_source))
+
+
+def test_outputs_type():
+    """Alert Processor Input Validation - Metadata Outputs Bad Type"""
+    # Default valid alert to be modified
+    invalid_metadata_outputs = _get_alert()
 
     # metadata > outputs type validation
-    invalid_metadata_outputs = deepcopy(valid_alert)
     invalid_metadata_outputs['metadata']['outputs'] = {'bad': 'value'}
 
     # Test with invalid metadata outputs type
     assert_false(validate_alert(invalid_metadata_outputs))
 
+
+def test_outputs_value_type():
+    """Alert Processor Input Validation - Metadata Outputs Bad Value Type"""
+    # Default valid alert to be modified
+    invalid_metadata_outputs = _get_alert()
+
     # metadata > outputs value validation
-    invalid_metadata_outputs_value = deepcopy(valid_alert)
-    invalid_metadata_outputs_value['metadata']['outputs'] = ['good', 100]
+    invalid_metadata_outputs['metadata']['outputs'] = ['good', 100]
 
     # Test with invalid metadata outputs value
-    assert_false(validate_alert(invalid_metadata_outputs_value))
+    assert_false(validate_alert(invalid_metadata_outputs))
+
+
+def test_metadata_non_string_type():
+    """Alert Processor Input Validation - Metadata Non-String"""
+    # Default valid alert to be modified
+    invalid_metadata_non_string = _get_alert()
 
     # metadata > non-string value validation
-    invalid_metadata_non_string = deepcopy(valid_alert)
     invalid_metadata_non_string['metadata']['type'] = 4.5
 
     # Test with invalid metadata non-string value