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

#110: Replace yaml loader with one that supports env value substitutions. #149

Merged
merged 1 commit into from
May 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
- Added support for alerting via Amazon Simple Email System (SES) - [#105](https://github.com/jertel/elastalert2/pull/105) - @nsano-rururu
- Support a footer in alert text - [#133](https://github.com/jertel/elastalert2/pull/133) - @nsano-rururu
- Support extra message features for Slack and Mattermost - [#140](https://github.com/jertel/elastalert2/pull/140) - @nsano-rururu
- Support for environment variable substitutions in yaml config files

## Other changes
- Fix issue with testing alerts that contain Jinja templates - [#101](https://github.com/jertel/elastalert2/pull/101) - @jertel
Expand Down
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@ test-elasticsearch:

test-docker:
docker-compose --project-name elastalert build tox
docker-compose --project-name elastalert run --rm tox
docker-compose --project-name elastalert run --rm tox tox -- $(filter-out $@,$(MAKECMDGOALS))
jertel marked this conversation as resolved.
Show resolved Hide resolved

clean:
make -C docs clean
find . -name '*.pyc' -delete
find . -name '__pycache__' -delete
rm -rf virtualenv_run .tox .coverage *.egg-info build

%:
jertel marked this conversation as resolved.
Show resolved Hide resolved
@:
4 changes: 2 additions & 2 deletions elastalert/alerts.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
from jira.exceptions import JIRAError
from requests.auth import HTTPProxyAuth
from requests.exceptions import RequestException
from staticconf.loader import yaml_loader
Copy link
Owner

Choose a reason for hiding this comment

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

With these staticconf imports eliminated, is there any reason to retain the PyStaticConfiguration>=0.10.3 lines in requirements.txt and setup.py? If it's no longer needed let's get rid of that dep.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can remove.

Copy link
Owner

Choose a reason for hiding this comment

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

@nsano-rururu Do you concur? You've spent a lot of time reviewing those dependencies so your opinion is appreciated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jertel Yes. is consent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jertel removed

from texttable import Texttable
from twilio.base.exceptions import TwilioRestException
from twilio.rest import Client as TwilioClient
Expand All @@ -38,6 +37,7 @@
from .util import resolve_string
from .util import ts_now
from .util import ts_to_dt
from .yaml import read_yaml


class DateTimeEncoder(json.JSONEncoder):
Expand Down Expand Up @@ -312,7 +312,7 @@ def get_account(self, account_file):
account_file_path = account_file
else:
account_file_path = os.path.join(os.path.dirname(self.rule['rule_file']), account_file)
account_conf = yaml_loader(account_file_path)
account_conf = read_yaml(account_file_path)
if 'user' not in account_conf or 'password' not in account_conf:
raise EAException('Account file must have user and password fields')
self.user = account_conf['user']
Expand Down
7 changes: 4 additions & 3 deletions elastalert/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
import logging.config

from envparse import Env
from staticconf.loader import yaml_loader

from . import loaders
from .util import EAException
from .util import elastalert_logger
from .util import get_module

from elastalert.yaml import read_yaml

# Required global (config.yaml) configuration options
required_globals = frozenset(['run_every', 'es_host', 'es_port', 'writeback_index', 'buffer_time'])

Expand Down Expand Up @@ -45,10 +46,10 @@ def load_conf(args, defaults=None, overwrites=None):
"""
filename = args.config
if filename:
conf = yaml_loader(filename)
conf = read_yaml(filename)
else:
try:
conf = yaml_loader('config.yaml')
conf = read_yaml('config.yaml')
except FileNotFoundError:
raise EAException('No --config or config.yaml found')

Expand Down
4 changes: 2 additions & 2 deletions elastalert/loaders.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from jinja2 import Template
from jinja2 import Environment
from jinja2 import FileSystemLoader
from staticconf.loader import yaml_loader

from . import alerts
from . import enhancements
Expand All @@ -29,6 +28,7 @@
from .util import unix_to_dt
from .util import unixms_to_dt
from .zabbix import ZabbixAlerter
from .yaml import read_yaml


class RulesLoader(object):
Expand Down Expand Up @@ -538,7 +538,7 @@ def get_hashes(self, conf, use_rule=None):

def get_yaml(self, filename):
try:
return yaml_loader(filename)
return read_yaml(filename)
except yaml.scanner.ScannerError as e:
raise EAException('Could not parse file %s: %s' % (filename, e))

Expand Down
8 changes: 8 additions & 0 deletions elastalert/yaml.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import os
import yaml


def read_yaml(path):
with open(path) as f:
yamlContent = os.path.expandvars(f.read())
return yaml.load(yamlContent, Loader=yaml.FullLoader)
1 change: 0 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ mock>=2.0.0
prison>=0.1.2
prometheus_client>=0.10.1
py-zabbix>=1.1.3
PyStaticConfiguration>=0.10.3
python-dateutil>=2.6.0,<2.9.0
PyYAML>=5.1
requests>=2.10.0
Expand Down
1 change: 0 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
'prison>=0.1.2',
'prometheus_client>=0.10.1',
'py-zabbix>=1.1.3',
'PyStaticConfiguration>=0.10.3',
'python-dateutil>=2.6.0,<2.9.0',
'PyYAML>=5.1',
'requests>=2.10.0',
Expand Down
24 changes: 12 additions & 12 deletions tests/alerts_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ def test_email_with_auth():
'alert_subject': 'Test alert for {0}', 'alert_subject_args': ['test_term'], 'smtp_auth_file': 'file.txt',
'rule_file': '/tmp/foo.yaml'}
with mock.patch('elastalert.alerts.SMTP') as mock_smtp:
with mock.patch('elastalert.alerts.yaml_loader') as mock_open:
with mock.patch('elastalert.alerts.read_yaml') as mock_open:
mock_open.return_value = {'user': 'someone', 'password': 'hunter2'}
mock_smtp.return_value = mock.Mock()
alert = EmailAlerter(rule)
Expand All @@ -226,7 +226,7 @@ def test_email_with_cert_key():
'alert_subject': 'Test alert for {0}', 'alert_subject_args': ['test_term'], 'smtp_auth_file': 'file.txt',
'smtp_cert_file': 'dummy/cert.crt', 'smtp_key_file': 'dummy/client.key', 'rule_file': '/tmp/foo.yaml'}
with mock.patch('elastalert.alerts.SMTP') as mock_smtp:
with mock.patch('elastalert.alerts.yaml_loader') as mock_open:
with mock.patch('elastalert.alerts.read_yaml') as mock_open:
mock_open.return_value = {'user': 'someone', 'password': 'hunter2'}
mock_smtp.return_value = mock.Mock()
alert = EmailAlerter(rule)
Expand Down Expand Up @@ -1279,7 +1279,7 @@ def test_jira():
mock_priority = mock.Mock(id='5')

with mock.patch('elastalert.alerts.JIRA') as mock_jira, \
mock.patch('elastalert.alerts.yaml_loader') as mock_open:
mock.patch('elastalert.alerts.read_yaml') as mock_open:
mock_open.return_value = {'user': 'jirauser', 'password': 'jirapassword'}
mock_jira.return_value.priorities.return_value = [mock_priority]
mock_jira.return_value.fields.return_value = []
Expand Down Expand Up @@ -1310,7 +1310,7 @@ def test_jira():
# Search called if jira_bump_tickets
rule['jira_bump_tickets'] = True
with mock.patch('elastalert.alerts.JIRA') as mock_jira, \
mock.patch('elastalert.alerts.yaml_loader') as mock_open:
mock.patch('elastalert.alerts.read_yaml') as mock_open:
mock_open.return_value = {'user': 'jirauser', 'password': 'jirapassword'}
mock_jira.return_value = mock.Mock()
mock_jira.return_value.search_issues.return_value = []
Expand All @@ -1326,7 +1326,7 @@ def test_jira():
# Remove a field if jira_ignore_in_title set
rule['jira_ignore_in_title'] = 'test_term'
with mock.patch('elastalert.alerts.JIRA') as mock_jira, \
mock.patch('elastalert.alerts.yaml_loader') as mock_open:
mock.patch('elastalert.alerts.read_yaml') as mock_open:
mock_open.return_value = {'user': 'jirauser', 'password': 'jirapassword'}
mock_jira.return_value = mock.Mock()
mock_jira.return_value.search_issues.return_value = []
Expand All @@ -1340,7 +1340,7 @@ def test_jira():

# Issue is still created if search_issues throws an exception
with mock.patch('elastalert.alerts.JIRA') as mock_jira, \
mock.patch('elastalert.alerts.yaml_loader') as mock_open:
mock.patch('elastalert.alerts.read_yaml') as mock_open:
mock_open.return_value = {'user': 'jirauser', 'password': 'jirapassword'}
mock_jira.return_value = mock.Mock()
mock_jira.return_value.search_issues.side_effect = JIRAError
Expand All @@ -1359,7 +1359,7 @@ def test_jira():
# Check ticket is bumped if it is updated 4 days ago
mock_issue.fields.updated = str(ts_now() - datetime.timedelta(days=4))
with mock.patch('elastalert.alerts.JIRA') as mock_jira, \
mock.patch('elastalert.alerts.yaml_loader') as mock_open:
mock.patch('elastalert.alerts.read_yaml') as mock_open:
mock_open.return_value = {'user': 'jirauser', 'password': 'jirapassword'}
mock_jira.return_value = mock.Mock()
mock_jira.return_value.search_issues.return_value = [mock_issue]
Expand All @@ -1375,7 +1375,7 @@ def test_jira():
# Check ticket is bumped is not bumped if ticket is updated right now
mock_issue.fields.updated = str(ts_now())
with mock.patch('elastalert.alerts.JIRA') as mock_jira, \
mock.patch('elastalert.alerts.yaml_loader') as mock_open:
mock.patch('elastalert.alerts.read_yaml') as mock_open:
mock_open.return_value = {'user': 'jirauser', 'password': 'jirapassword'}
mock_jira.return_value = mock.Mock()
mock_jira.return_value.search_issues.return_value = [mock_issue]
Expand Down Expand Up @@ -1410,7 +1410,7 @@ def test_jira():
{'name': 'affected user', 'id': 'affected_user_id', 'schema': {'type': 'string'}}
]
with mock.patch('elastalert.alerts.JIRA') as mock_jira, \
mock.patch('elastalert.alerts.yaml_loader') as mock_open:
mock.patch('elastalert.alerts.read_yaml') as mock_open:
mock_open.return_value = {'user': 'jirauser', 'password': 'jirapassword'}
mock_jira.return_value = mock.Mock()
mock_jira.return_value.search_issues.return_value = [mock_issue]
Expand Down Expand Up @@ -1483,7 +1483,7 @@ def test_jira_arbitrary_field_support():
]

with mock.patch('elastalert.alerts.JIRA') as mock_jira, \
mock.patch('elastalert.alerts.yaml_loader') as mock_open:
mock.patch('elastalert.alerts.read_yaml') as mock_open:
mock_open.return_value = {'user': 'jirauser', 'password': 'jirapassword'}
mock_jira.return_value.priorities.return_value = [mock_priority]
mock_jira.return_value.fields.return_value = mock_fields
Expand Down Expand Up @@ -1524,7 +1524,7 @@ def test_jira_arbitrary_field_support():
rule['jira_nonexistent_field'] = 'nonexistent field value'

with mock.patch('elastalert.alerts.JIRA') as mock_jira, \
mock.patch('elastalert.alerts.yaml_loader') as mock_open:
mock.patch('elastalert.alerts.read_yaml') as mock_open:
mock_open.return_value = {'user': 'jirauser', 'password': 'jirapassword'}
mock_jira.return_value.priorities.return_value = [mock_priority]
mock_jira.return_value.fields.return_value = mock_fields
Expand All @@ -1540,7 +1540,7 @@ def test_jira_arbitrary_field_support():
rule['jira_watchers'] = 'invalid_watcher'

with mock.patch('elastalert.alerts.JIRA') as mock_jira, \
mock.patch('elastalert.alerts.yaml_loader') as mock_open:
mock.patch('elastalert.alerts.read_yaml') as mock_open:
mock_open.return_value = {'user': 'jirauser', 'password': 'jirapassword'}
mock_jira.return_value.priorities.return_value = [mock_priority]
mock_jira.return_value.fields.return_value = mock_fields
Expand Down
34 changes: 34 additions & 0 deletions tests/config_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# -*- coding: utf-8 -*-
import os
import mock
import datetime

from elastalert.config import load_conf


def test_config_loads():
os.environ['ELASTIC_PASS'] = 'password_from_env'
dir_path = os.path.dirname(os.path.realpath(__file__))

test_args = mock.Mock()
test_args.config = dir_path + '/example.config.yaml'
test_args.rule = None
test_args.debug = False
test_args.es_debug_trace = None

conf = load_conf(test_args)

assert conf['rules_folder'] == '/opt/elastalert/rules'
assert conf['run_every'] == datetime.timedelta(seconds=10)
assert conf['buffer_time'] == datetime.timedelta(minutes=15)

assert conf['es_host'] == 'elasticsearch'
assert conf['es_port'] == 9200

assert conf['es_username'] == 'elastic'
assert conf['es_password'] == 'password_from_env'

assert conf['writeback_index'] == 'elastalert_status'
assert conf['writeback_alias'] == 'elastalert_alerts'

assert conf['alert_time_limit'] == datetime.timedelta(days=2)
19 changes: 19 additions & 0 deletions tests/example.config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
rules_folder: /opt/elastalert/rules

run_every:
seconds: 10

buffer_time:
minutes: 15

es_host: elasticsearch
es_port: 9200

es_username: elastic
es_password: $ELASTIC_PASS

writeback_index: elastalert_status
writeback_alias: elastalert_alerts

alert_time_limit:
days: 2
28 changes: 14 additions & 14 deletions tests/loaders_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,9 @@ def test_file_rules_loader_get_names():
def test_load_rules():
test_rule_copy = copy.deepcopy(test_rule)
test_config_copy = copy.deepcopy(test_config)
with mock.patch('elastalert.config.yaml_loader') as mock_conf_open:
with mock.patch('elastalert.config.read_yaml') as mock_conf_open:
mock_conf_open.return_value = test_config_copy
with mock.patch('elastalert.loaders.yaml_loader') as mock_rule_open:
with mock.patch('elastalert.loaders.read_yaml') as mock_rule_open:
mock_rule_open.return_value = test_rule_copy

with mock.patch('os.walk') as mock_ls:
Expand All @@ -233,9 +233,9 @@ def test_load_default_host_port():
test_rule_copy.pop('es_host')
test_rule_copy.pop('es_port')
test_config_copy = copy.deepcopy(test_config)
with mock.patch('elastalert.config.yaml_loader') as mock_conf_open:
with mock.patch('elastalert.config.read_yaml') as mock_conf_open:
mock_conf_open.return_value = test_config_copy
with mock.patch('elastalert.loaders.yaml_loader') as mock_rule_open:
with mock.patch('elastalert.loaders.read_yaml') as mock_rule_open:
mock_rule_open.return_value = test_rule_copy

with mock.patch('os.walk') as mock_ls:
Expand All @@ -253,9 +253,9 @@ def test_load_ssl_env_false():
test_rule_copy.pop('es_host')
test_rule_copy.pop('es_port')
test_config_copy = copy.deepcopy(test_config)
with mock.patch('elastalert.config.yaml_loader') as mock_conf_open:
with mock.patch('elastalert.config.read_yaml') as mock_conf_open:
mock_conf_open.return_value = test_config_copy
with mock.patch('elastalert.loaders.yaml_loader') as mock_rule_open:
with mock.patch('elastalert.loaders.read_yaml') as mock_rule_open:
mock_rule_open.return_value = test_rule_copy

with mock.patch('os.listdir') as mock_ls:
Expand All @@ -272,9 +272,9 @@ def test_load_ssl_env_true():
test_rule_copy.pop('es_host')
test_rule_copy.pop('es_port')
test_config_copy = copy.deepcopy(test_config)
with mock.patch('elastalert.config.yaml_loader') as mock_conf_open:
with mock.patch('elastalert.config.read_yaml') as mock_conf_open:
mock_conf_open.return_value = test_config_copy
with mock.patch('elastalert.loaders.yaml_loader') as mock_rule_open:
with mock.patch('elastalert.loaders.read_yaml') as mock_rule_open:
mock_rule_open.return_value = test_rule_copy

with mock.patch('os.listdir') as mock_ls:
Expand All @@ -291,9 +291,9 @@ def test_load_url_prefix_env():
test_rule_copy.pop('es_host')
test_rule_copy.pop('es_port')
test_config_copy = copy.deepcopy(test_config)
with mock.patch('elastalert.config.yaml_loader') as mock_conf_open:
with mock.patch('elastalert.config.read_yaml') as mock_conf_open:
mock_conf_open.return_value = test_config_copy
with mock.patch('elastalert.loaders.yaml_loader') as mock_rule_open:
with mock.patch('elastalert.loaders.read_yaml') as mock_rule_open:
mock_rule_open.return_value = test_rule_copy

with mock.patch('os.listdir') as mock_ls:
Expand All @@ -309,9 +309,9 @@ def test_load_disabled_rules():
test_rule_copy = copy.deepcopy(test_rule)
test_rule_copy['is_enabled'] = False
test_config_copy = copy.deepcopy(test_config)
with mock.patch('elastalert.config.yaml_loader') as mock_conf_open:
with mock.patch('elastalert.config.read_yaml') as mock_conf_open:
mock_conf_open.return_value = test_config_copy
with mock.patch('elastalert.loaders.yaml_loader') as mock_rule_open:
with mock.patch('elastalert.loaders.read_yaml') as mock_rule_open:
mock_rule_open.return_value = test_rule_copy

with mock.patch('os.listdir') as mock_ls:
Expand All @@ -334,9 +334,9 @@ def test_raises_on_missing_config():
if key in optional_keys:
continue

with mock.patch('elastalert.config.yaml_loader') as mock_conf_open:
with mock.patch('elastalert.config.read_yaml') as mock_conf_open:
mock_conf_open.return_value = test_config_copy
with mock.patch('elastalert.loaders.yaml_loader') as mock_rule_open:
with mock.patch('elastalert.loaders.read_yaml') as mock_rule_open:
mock_rule_open.return_value = test_rule_copy
with mock.patch('os.walk') as mock_walk:
mock_walk.return_value = [('', [], ['testrule.yaml'])]
Expand Down