Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make list targets work again #1210

Merged
merged 3 commits into from
Mar 26, 2020
Merged

Conversation

chunyong-lin
Copy link
Contributor

to: @airbnb/streamalert-maintainers
related to:
resolves:

Background

Deploy a fresh copy of release-3-1-0 branch to staging environment, for, fun 🤷‍♀ and found couple minor bugs.

Changes

  • python manage.py list-targets command is broken due to recent change of terraform files path.
  • Add use test cases to test the code change.
  • Add a space when print out WARNING messing to remind user to update file_format setting.
  • Add a Note in getting_started doc to remind user to update file_format setting.
    image

Testing

  • Fresh checkout release-3-1-0 branch
  • python manage.py configure aws_account_id 111111111111
  • python manage.py configure prefix cylinrelease31
  • python manage.py init
  • A ConfigError warming to set "file_format" to "parquet" in "athena_partition_refresh_config" raised.
  • Update "file_format" to "parquet" and re-init
  • python manage.py init and StreamAlert initialization was successful.
  • Create a kinesis stream in cluster "prod"
  "modules": {
    "cloudwatch_monitoring": {
      "enabled": true,
      "kinesis_alarms_enabled": false,
      "lambda_alarms_enabled": true
    },
    "kinesis": {
      "streams": {
        "create_user": true,
        "retention": 24,
        "shards": 1,
        "terraform_outputs": [
          "arn",
          "access_key_id",
          "secret_key"
        ]
      }
    },
    "kinesis_events": {
      "batch_size": 100,
      "enabled": true
    }
  }
  • Add new kinesis name to the "data_sources" filed in conf/clusters/prod.json
  "data_sources": {
    "kinesis": {
      "prefix_cluster1_streamalert": [
        "cloudwatch",
        "ghe",
        "osquery"
      ],
      "cylinrelease31_prod_streamalert": [
        "cloudwatch",
        "osquery"
      ]
    },
  • Run build and deploy "prod" classifier
python manage.py build --target kinesis_events_prod kinesis_prod
python manage.py deploy --function classifier
  • Send testing cloudwatch events to the kinesis to trigger classifier, rules engine, alert processor, alert merger.
  • Enable firehose and run build to create firehose and athena tables for cloudwatch events. in conf/global.json
  "infrastructure": {
    "alerts_table": {
      "read_capacity": 5,
      "write_capacity": 5
    },
    "firehose": {
      "use_prefix": true,
      "buffer_interval": 900,
      "buffer_size": 128,
      "enabled": true,
      "enabled_logs": {
        "cloudwatch": {},
        "osquery": {}
      }
    },
  • python manage.py build
  • python manage.py deploy --function classifier
  • python manage.py deploy --function athena
  • Send testing cloudwatch events to the kinesis to trigger classifier, rules engine, alert processor, alert merger and athena partition lambda function.
  • Alerts and cloudWatch events are searchable in Athena table/

@coveralls
Copy link

coveralls commented Mar 25, 2020

Coverage Status

Coverage remained the same at 95.424% when pulling 4972858 on make_list_targets_work_again into 530a154 on release-3-1-0.

@@ -103,6 +103,20 @@ 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``, choose ``parquet`` or ``json``. We will set default value to ``parquet`` in the future release.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update to:

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.

"log_level": "info"
}

* For more information, please visit :ref:`historical_search` page for more information.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is redundant.. For more information .... for more information

Please update to:

More information can be found on the :ref:historical_search page

Copy link
Contributor

@ryandeivert ryandeivert Mar 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually -- can we stop using ref items so much. they're ugly an unnecessary most of the time

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you update this to:

More information can be found on the `historical search <historical-search.html>`_ page.

see example here: https://raw.githubusercontent.com/airbnb/streamalert/530a154fd446a133868a49997bfbb47deb598c82/docs/source/datasources.rst:

To configure datasources, read `datasource configuration <conf-datasources.html>`_

@@ -1,4 +1,5 @@
#################
.. _historical_search:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove this

@@ -409,7 +409,7 @@ def get_tf_modules(config, generate=False):

modules = set()
resources = set()
for root, _, files in os.walk('terraform'):
for root, _, files in os.walk(TERRAFORM_FILES_PATH):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good find thank youuu!

Copy link
Contributor

@ryandeivert ryandeivert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some comments, please address - lgtm otherwise. thank you so much for adding tests!!

@@ -618,7 +618,7 @@ def generate_global_lambda_settings(config, config_name, generate_func, tf_tmp_f
'It is required to explicitly set "file_format" for '
'athena_partition_refresh_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'
'https://github.com/airbnb/streamalert/issues/1143. '
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the trailing space here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a space between ....issues/1143 and In the future release. The message is in one line.

@chunyong-lin chunyong-lin merged commit 7a14dcf into release-3-1-0 Mar 26, 2020
@chunyong-lin chunyong-lin deleted the make_list_targets_work_again branch March 26, 2020 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants