addressing PR feedback

streamalert/shared/athena.py

@@ -59,7 +59,7 @@ def __init__(self, database_name, results_bucket, results_prefix, region=None):
         if not results_bucket.startswith('s3://'):
             results_bucket = 's3://{}'.format(results_bucket)
 
-        # Produces athena_partitioner/YYYY/MM/DD S3 keys
+        # Produces s3://<results_bucket_name>/<results_prefix>/YYYY/MM/DD S3 keys
         self._s3_results_path = posixpath.join(
             results_bucket,
             results_prefix,

streamalert_cli/terraform/generate.py

@@ -457,58 +457,54 @@ def terraform_generate_handler(config, init=False, check_tf=True, check_creds=Tr
             os.path.join(TERRAFORM_FILES_PATH, 'metric_alarms.tf.json')
         )
 
-    # Setup Athena
-    generate_global_lambda_settings(
-        config,
-        config_name='athena_partitioner_config',
-        generate_func=generate_athena,
-        tf_tmp_file=os.path.join(TERRAFORM_FILES_PATH, 'athena.tf.json'),
-        message='Removing old Athena Terraform file'
-    )
-
     # Setup Threat Intel Downloader Lambda function if it is enabled
     generate_global_lambda_settings(
         config,
-        config_name='threat_intel_downloader_config',
+        conf_name='threat_intel_downloader_config',
         generate_func=generate_threat_intel_downloader,
-        tf_tmp_file=os.path.join(TERRAFORM_FILES_PATH, 'ti_downloader.tf.json'),
-        message='Removing old Threat Intel Downloader Terraform file'
+        tf_tmp_file_name='ti_downloader',
+        required=False,
     )
 
     # Setup Rule Promotion if it is enabled
     generate_global_lambda_settings(
         config,
-        config_name='rule_promotion_config',
+        conf_name='rule_promotion_config',
         generate_func=generate_rule_promotion,
-        tf_tmp_file=os.path.join(TERRAFORM_FILES_PATH, 'rule_promotion.tf.json'),
-        message='Removing old Rule Promotion Terraform file'
+        tf_tmp_file_name='rule_promotion',
+        required=False,
+    )
+
+    # Setup Athena Partitioner
+    generate_global_lambda_settings(
+        config,
+        conf_name='athena_partitioner_config',
+        generate_func=generate_athena,
+        tf_tmp_file_name='athena',
     )
 
     # Setup Rules Engine
     generate_global_lambda_settings(
         config,
-        config_name='rules_engine_config',
+        conf_name='rules_engine_config',
         generate_func=generate_rules_engine,
-        tf_tmp_file=os.path.join(TERRAFORM_FILES_PATH, 'rules_engine.tf.json'),
-        message='Removing old Rules Engine Terraform file'
+        tf_tmp_file_name='rules_engine',
     )
 
     # Setup Alert Processor
     generate_global_lambda_settings(
         config,
-        config_name='alert_processor_config',
+        conf_name='alert_processor_config',
         generate_func=generate_alert_processor,
-        tf_tmp_file=os.path.join(TERRAFORM_FILES_PATH, 'alert_processor.tf.json'),
-        message='Removing old Alert Processor Terraform file'
+        tf_tmp_file_name='alert_processor',
     )
 
     # Setup Alert Merger
     generate_global_lambda_settings(
         config,
-        config_name='alert_merger_config',
+        conf_name='alert_merger_config',
         generate_func=generate_alert_merger,
-        tf_tmp_file=os.path.join(TERRAFORM_FILES_PATH, 'alert_merger.tf.json'),
-        message='Removing old Alert Merger Terraform file'
+        tf_tmp_file_name='alert_merger',
     )
 
     # Setup Lookup Tables if applicable
@@ -527,7 +523,7 @@ def _generate_lookup_tables_settings(config):
     tf_file_name = os.path.join(TERRAFORM_FILES_PATH, 'lookup_tables.tf.json')
 
     if not config['lookup_tables'].get('enabled', False):
-        remove_temp_terraform_file(tf_file_name, 'Removing old LookupTables Terraform file')
+        remove_temp_terraform_file(tf_file_name)
         return
 
     # Use the lookup_tables.json configuration file to determine which resources we have
@@ -545,7 +541,7 @@ def _generate_lookup_tables_settings(config):
     if not dynamodb_tables and not s3_buckets:
         # If no resources are configured at all, simply return and do not generate lookuptables
         # IAM policies
-        remove_temp_terraform_file(tf_file_name, 'No tables configured')
+        remove_temp_terraform_file(tf_file_name, extra='No tables configured')
         return
 
     roles = {
@@ -588,7 +584,7 @@ def _generate_streamquery_module(config):
     """
     tf_file_name = os.path.join(TERRAFORM_FILES_PATH, 'scheduled_queries.tf.json')
     if not config.get('scheduled_queries', {}).get('enabled', False):
-        remove_temp_terraform_file(tf_file_name, 'Removing old scheduled queries Terraform file')
+        remove_temp_terraform_file(tf_file_name)
         return
 
     _create_terraform_module_file(
@@ -597,7 +593,12 @@ def _generate_streamquery_module(config):
     )
 
 
-def generate_global_lambda_settings(config, config_name, generate_func, tf_tmp_file, message):
+def generate_global_lambda_settings(
+        config,
+        conf_name,
+        generate_func,
+        tf_tmp_file_name,
+        required=True):
     """Generate settings for global Lambda functions
 
     Args:
@@ -607,7 +608,7 @@ def generate_global_lambda_settings(config, config_name, generate_func, tf_tmp_f
         tf_tmp_file (str): filename of terraform file, generated by CLI.
         message (str): Message will be logged by LOGGER.
     """
-    if config_name == 'athena_partitioner_config':
+    if conf_name == 'athena_partitioner_config':
         # Raise ConfigError when user doesn't explicitly set `file_format`
         # in `athena_partitioner_config` in conf/lambda.json when upgrade to v3.1.0.
         file_format = get_data_file_format(config)
@@ -624,26 +625,36 @@ def generate_global_lambda_settings(config, config_name, generate_func, tf_tmp_f
             )
             raise ConfigError(message)
 
-    if not config['lambda'].get(config_name):
-        LOGGER.warning('Config for \'%s\' not in lambda.json', config_name)
-        remove_temp_terraform_file(tf_tmp_file, message)
+    tf_tmp_file = os.path.join(TERRAFORM_FILES_PATH, '{}.tf.json'.format(tf_tmp_file_name))
+
+    if required and conf_name not in config['lambda']:
+        message = 'Required configuration missing in lambda.json: {}'.format(conf_name)
+        raise ConfigError(message)
+
+    if not config['lambda'].get(conf_name):
+        LOGGER.warning('Optional configuration missing in lambda.json, skipping: %s', conf_name)
+        remove_temp_terraform_file(tf_tmp_file)
         return
 
-    if config['lambda'][config_name].get('enabled', True):
+    if config['lambda'][conf_name].get('enabled', True):
         generated_config = generate_func(config=config)
         if generated_config:
             _create_terraform_module_file(generated_config, tf_tmp_file)
     else:
-        remove_temp_terraform_file(tf_tmp_file, message)
+        remove_temp_terraform_file(tf_tmp_file)
 
 
-def remove_temp_terraform_file(tf_tmp_file, message):
+def remove_temp_terraform_file(tf_tmp_file, extra=None):
     """Remove temporal terraform file
 
     Args:
         tf_tmp_file (str): filename of terraform file, generated by CLI.
         message (str): Message will be logged by LOGGER.
     """
+    if extra:
+        LOGGER.info(extra)
+
+    message = 'Removing old Terraform file: {}'.format(tf_tmp_file)
     if os.path.isfile(tf_tmp_file):
         LOGGER.info(message)
         os.remove(tf_tmp_file)

tests/unit/streamalert_cli/terraform/test_generate.py

@@ -783,30 +783,56 @@ def test_generate_main_with_sqs_url_false(self):
         assert_equal(result['module']['globals']['source'], './modules/tf_globals')
         assert_false(result['module']['globals']['sqs_use_prefix'])
 
-    def test_generate_main_file_format_unspecified(self):
-        "CLI - Terraform Generate Main raises error when file_format unspecified"
+    def test_generate_athena_lambda_format_unspecified(self):
+        "CLI - Terraform Generate Global Lambda Settings, Unspecified Athena file_format"
         self.config['lambda']['athena_partitioner_config']['file_format'] = None
 
         assert_raises(
             ConfigError,
             generate.generate_global_lambda_settings,
             config=self.config,
-            config_name='athena_partitioner_config',
+            conf_name='athena_partitioner_config',
             generate_func='test_func',
-            tf_tmp_file='test_tf_tmp_file_path',
-            message='test message'
+            tf_tmp_file_name='test_tf_tmp_file_path',
         )
 
-    def test_generate_main_file_format_misconfigured(self):
-        "CLI - Terraform Generate Main raises error when file_format misconfigured"
+    def test_generate_athena_lambda_format_invalid(self):
+        "CLI - Terraform Generate Global Lambda Settings, Invalid Athena file_format"
         self.config['lambda']['athena_partitioner_config']['file_format'] = 'Parquet'
 
         assert_raises(
             ConfigError,
             generate.generate_global_lambda_settings,
             config=self.config,
-            config_name='athena_partitioner_config',
+            conf_name='athena_partitioner_config',
             generate_func='test_func',
-            tf_tmp_file='test_tf_tmp_file_path',
-            message='test message'
+            tf_tmp_file_name='test_tf_tmp_file_path',
+        )
+
+    def test_generate_required_lambda_invalid_config(self):
+        "CLI - Terraform Generate Global Lambda Settings, Invalid Config"
+
+        assert_raises(
+            ConfigError,
+            generate.generate_global_lambda_settings,
+            config=self.config,
+            conf_name='athena_partition_refresh_config',
+            generate_func='test_func',
+            tf_tmp_file_name='test_tf_tmp_file_path',
+        )
+
+    @patch('logging.Logger.warning')
+    def test_generate_optional_lambda_not_in_config(self, log_mock):
+        "CLI - Terraform Generate Global Lambda Settings, Optional Missing in Config"
+        fake_opt_conf_name = 'fake_optional_conf_name'
+        generate.generate_global_lambda_settings(
+            config=self.config,
+            conf_name=fake_opt_conf_name,
+            generate_func='test_func',
+            tf_tmp_file_name='test_tf_tmp_file_path',
+            required=False,
+        )
+
+        log_mock.assert_called_with(
+            'Optional configuration missing in lambda.json, skipping: %s', fake_opt_conf_name
         )