Skip to content

Commit

Permalink
adding fix for #1294
Browse files Browse the repository at this point in the history
  • Loading branch information
ryandeivert committed Oct 5, 2020
1 parent fbcde0b commit dc15684
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 81 deletions.
2 changes: 1 addition & 1 deletion conf/lambda.json
Expand Up @@ -54,7 +54,7 @@
"concurrency_limit": 10,
"memory": 128,
"timeout": 300,
"file_format": null,
"file_format": "parquet",
"log_level": "info"
},
"classifier_config": {},
Expand Down
4 changes: 0 additions & 4 deletions docs/source/getting-started.rst
Expand Up @@ -103,10 +103,6 @@ Deploy
python manage.py configure aws_account_id 111111111111 # Replace with your 12-digit AWS account ID
python manage.py configure prefix <value> # Choose a unique name prefix (alphanumeric characters only)
.. note::

* Update the ``file_format`` value in ``conf/lambda.json``. Valid options are ``parquet`` or ``json``. The default value will be parquet in a future release, but this must be manually configured at this time.

.. code-block:: bash
"athena_partitioner_config": {
Expand Down
26 changes: 21 additions & 5 deletions streamalert_cli/config.py
Expand Up @@ -23,11 +23,12 @@
from streamalert.apps import StreamAlertApp
from streamalert.shared import CLUSTERED_FUNCTIONS, config, metrics
from streamalert.shared.logger import get_logger
from streamalert_cli.helpers import continue_prompt
from streamalert_cli.terraform import TERRAFORM_FILES_PATH
from streamalert_cli.apps.helpers import save_app_auth_info
from streamalert_cli.helpers import continue_prompt

DEFAULT_CONFIG_PATH = 'conf'

DEFAULT_CONFIG_PATH = 'conf'
LOGGER = get_logger(__name__)


Expand Down Expand Up @@ -69,8 +70,21 @@ def terraform_files(self):
self.config['global']['general'].get('terraform_files', [])
)

@staticmethod
def _setup_build_directory(directory):
def _copy_terraform_files(self, directory):
"""Copy all packaged terraform files and terraform files provided by the user to temp
Args:
config (CLIConfig): Loaded StreamAlert config
"""
shutil.copytree(TERRAFORM_FILES_PATH, directory)

# Copy any additional user provided terraform files to temp
for item in self.terraform_files:
shutil.copy2(item, directory)

LOGGER.info('Copied Terraform configuration to \'%s\'', directory)

def _setup_build_directory(self, directory):
"""Create the directory to be used for building infrastructure
Args:
Expand All @@ -87,7 +101,9 @@ def _setup_build_directory(directory):
temp_dir.cleanup()

if os.path.exists(directory):
shutil.rmtree(directory)
shutil.rmtree(directory) # shutil.copytree in python3.7 cannot handle existing dir

self._copy_terraform_files(directory)

return directory

Expand Down
41 changes: 0 additions & 41 deletions streamalert_cli/terraform/generate.py
Expand Up @@ -384,28 +384,6 @@ def handler(cls, options, config):
return terraform_generate_handler(config, check_creds=False)


def _copy_terraform_files(config):
"""Copy all packaged terraform files and terraform files provided by the user to temp
Args:
config (CLIConfig): Loaded StreamAlert config
"""
# Copy the packaged terraform files to temp
# Currently this ignores *.tf.json, in the instance that these
# exist in current deployments. This can be removed in a future release.
shutil.copytree(
TERRAFORM_FILES_PATH,
config.build_directory,
ignore=shutil.ignore_patterns('*.tf.json') # TODO: remove this eventually
)

# Copy any additional user provided terraform files to temp
for item in config.terraform_files:
shutil.copy2(item, config.build_directory)

LOGGER.info('Copied Terraform configuration to \'%s\'', config.build_directory)


def terraform_generate_handler(config, init=False, check_tf=True, check_creds=True):
"""Generate all Terraform plans for the configured clusters.
Expand All @@ -424,8 +402,6 @@ def terraform_generate_handler(config, init=False, check_tf=True, check_creds=Tr
if check_tf and not terraform_check():
return False

_copy_terraform_files(config)

# Setup the main.tf.json file
LOGGER.debug('Generating cluster file: main.tf.json')
_create_terraform_module_file(
Expand Down Expand Up @@ -634,23 +610,6 @@ def generate_global_lambda_settings(
tf_tmp_file (str): filename of terraform file, generated by CLI.
message (str): Message will be logged by LOGGER.
"""
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)

if not file_format or file_format not in ('parquet', 'json'):
message = (
'[WARNING] '
'It is required to explicitly set "file_format" for '
'athena_partitioner_config in "conf/lambda.json" when upgrading to v3.1.0. '
'Available values are "parquet" and "json". For more information, refer to '
'https://github.com/airbnb/streamalert/issues/1143. '
'In the future release, the default value of "file_format" will '
'be changed to "parquet".'
)
raise ConfigError(message)

tf_tmp_file = os.path.join(config.build_directory, '{}.tf.json'.format(tf_tmp_file_name))

if required and conf_name not in config['lambda']:
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/streamalert_cli/manage_lambda/test_package.py
Expand Up @@ -16,7 +16,7 @@
# pylint: disable=no-self-use,protected-access
import os

from mock import patch
from mock import patch, Mock
from pyfakefs import fake_filesystem_unittest

from streamalert_cli.config import CLIConfig
Expand All @@ -28,6 +28,7 @@ class PackageTest(fake_filesystem_unittest.TestCase):
TEST_CONFIG_PATH = 'tests/unit/conf'
MOCK_TEMP_PATH = '/tmp/test_packaging'

@patch('streamalert_cli.config.CLIConfig._copy_terraform_files', Mock())
def setUp(self):
self.setUpPyfakefs()
self.fs.add_real_directory(self.TEST_CONFIG_PATH)
Expand Down
3 changes: 1 addition & 2 deletions tests/unit/streamalert_cli/terraform/test_firehose.py
Expand Up @@ -218,7 +218,6 @@ def test_firehose_enabled_log_json(self):
'json:embedded': {}
}

self.config = CLIConfig(config_path='tests/unit/conf_athena')
firehose.generate_firehose(self._logging_bucket_name, cluster_dict, self.config)

expected_result = {
Expand All @@ -228,7 +227,7 @@ def test_firehose_enabled_log_json(self):
'source': './modules/tf_kinesis_firehose_delivery_stream',
'buffer_size': 128,
'buffer_interval': 900,
'file_format': 'json',
'file_format': 'parquet',
'stream_name': 'unit_test_streamalert_json_embedded',
'role_arn': '${module.kinesis_firehose_setup.firehose_role_arn}',
's3_bucket_name': 'unit-test-streamalert-data',
Expand Down
26 changes: 0 additions & 26 deletions tests/unit/streamalert_cli/terraform/test_generate.py
Expand Up @@ -854,32 +854,6 @@ 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_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,
conf_name='athena_partitioner_config',
generate_func='test_func',
tf_tmp_file_name='test_tf_tmp_file_path',
)

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,
conf_name='athena_partitioner_config',
generate_func='test_func',
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"

Expand Down
3 changes: 2 additions & 1 deletion tests/unit/streamalert_cli/test_cli_config.py
Expand Up @@ -16,7 +16,7 @@
# pylint: disable=protected-access
import json

from mock import patch
from mock import patch, Mock
from nose.tools import assert_equal, assert_true, assert_false
from pyfakefs import fake_filesystem_unittest

Expand All @@ -31,6 +31,7 @@ def __init__(self):
self.config = None
self.fs_patcher = None

@patch('streamalert_cli.config.CLIConfig._copy_terraform_files', Mock())
def setup(self):
"""Setup before each method"""
config_data = basic_streamalert_config()
Expand Down

0 comments on commit dc15684

Please sign in to comment.