Merge pull request #54 from jantman/issues/53

Fixes #53 - mugc not respecting function_prefix

CHANGES.rst

@@ -1,5 +1,17 @@
 Changelog
 =========
+
+1.2.4 (2020-07-29)
+------------------
+
+* Fixes `#53 <https://github.com/manheim/manheim-c7n-tools/issues/53>`__
+
+  * Add ``function_prefix`` option to ``manheim-c7n-tools.yml`` to allow passing this option to mugc. Default it to the current/default ``custodian-``.
+  * Have :py:class:`~.runner.MugcStep` use configured ``function_prefix`` instead of hard-coded ``custodian-``.
+  * New policy sanity check :py:meth:`~.PolicyGen._check_policy_function_prefix` - fail if a policy's ``function-prefix`` doesn't match the configured (``manheim-c7n-tools.yml``) ``function_prefix``.
+
+* Switch from deprecated pep8 / pytest-pep8 to pycodestyle / pytest-pycodestyle.
+
 1.2.3 (2020-07-10)
 ------------------
 

manheim_c7n_tools/config.py

@@ -56,6 +56,8 @@
         # Optional policy source paths. If not specified, uses the current
         # directory
         'policy_source_paths': {'type': 'array', 'items': {'type': 'string'}},
+        # Name prefix for custodian Lambda functions
+        'function_prefix': {'type': 'string'},
         # A list of region names that custodian should run in for this account
         'regions': {'type': 'array', 'items': {'type': 'string'}},
         # Name of the S3 bucket for storing Custodian output; should include
@@ -72,7 +74,6 @@
         # Array of notification recipients for orphaned Lambda/CWE Rule
         # notifications; set to empty array to disable this functionality
         'cleanup_notify': {'type': 'array'},
-
         # Optional list of notification targets to add to EVERY policy
         'always_notify': {
             'to': {'type': 'array', 'items': {'type': 'string'}},
@@ -115,9 +116,11 @@ class ManheimConfig(object):
     def __init__(self, **kwargs):
         self.config_path = kwargs.pop('config_path')
         logger.debug('Validating configuration...')
-        jsonschema.validate(kwargs, MANHEIM_CONFIG_SCHEMA)
+        jsonschema.validate(dict(kwargs), MANHEIM_CONFIG_SCHEMA)
         self._config = kwargs
         self._config['account_id'] = str(self._config['account_id'])
+        if 'function_prefix' not in self._config:
+            self._config['function_prefix'] = 'custodian-'
         if 'cleanup_notify' not in self._config:
             self._config['cleanup_notify'] = []
 

manheim_c7n_tools/errorscan.py

@@ -576,9 +576,9 @@ def _check_function(self, func_name, never_match_re=None):
             )))
             for e in events:
                 print("\n".join([
-                    "\t\t%s" % l.replace("\t", ' ')
-                    for l in e['message'].split("\n")
-                    if l.strip() != ''
+                    "\t\t%s" % line.replace("\t", ' ')
+                    for line in e['message'].split("\n")
+                    if line.strip() != ''
                 ]))
         if 'always_match' in logs:
             print("\t" + red(
@@ -587,21 +587,21 @@ def _check_function(self, func_name, never_match_re=None):
             ))
             for e in logs['always_match']:
                 print("\n".join([
-                    "\t\t%s" % l.replace("\t", ' ')
-                    for l in e['message'].split("\n")
-                    if l.strip() != ''
+                    "\t\t%s" % line.replace("\t", ' ')
+                    for line in e['message'].split("\n")
+                    if line.strip() != ''
                 ]))
         print('')
         return False
 
 
-def _name_value_dict(l):
+def _name_value_dict(lst):
     """
-    Given a list (``l``) containing dicts with ``Name`` and ``Value`` keys,
+    Given a list (``lst``) containing dicts with ``Name`` and ``Value`` keys,
     return a single dict of Name -> Value.
     """
     res = {}
-    for item in l:
+    for item in lst:
         res[item['Name']] = item['Value']
     return res
 

manheim_c7n_tools/policygen.py

@@ -329,6 +329,16 @@ def _check_policies(self, policies):
             raise SystemExit(1)
         logger.info('OK: All policies passed sanity/safety checks.')
 
+    def _check_policy_function_prefix(self, policy):
+        """
+        Fail if function-prefix doesn't match between manheim-c7n-tools config
+        and the policy.
+        """
+        fp = policy.get('mode', {}).get('function-prefix', 'custodian-')
+        if fp != self._config.function_prefix:
+            return False
+        return True
+
     def _check_policy_marked_for_op_first(self, policy):
         """
         Policy includes a marked-for-op filter, but it is not the first filter.

manheim_c7n_tools/runner.py

@@ -31,6 +31,7 @@
 from shutil import rmtree
 import os
 from copy import deepcopy
+import re
 
 from sphinx.cmd.build import main as sphinx_main
 import jsonschema
@@ -181,8 +182,8 @@ def run(self):
         conf = Config.empty(
             config_files=['custodian_%s.yml' % self.region_name],
             regions=[self.region_name],
-            prefix='custodian-',
-            policy_regex='^custodian-.*',
+            prefix=self.config.function_prefix,
+            policy_regex='^' + re.escape(self.config.function_prefix) + '.*',
             assume=None,
             policy_filter=None,
             log_group=None,
@@ -212,8 +213,8 @@ def dryrun(self):
         conf = Config.empty(
             config_files=['custodian_%s.yml' % self.region_name],
             regions=[self.region_name],
-            prefix='custodian-',
-            policy_regex='^custodian-.*',
+            prefix=self.config.function_prefix,
+            policy_regex='^' + re.escape(self.config.function_prefix) + '.*',
             assume=None,
             policy_filter=None,
             log_group=None,

manheim_c7n_tools/tests/test_config.py

@@ -35,7 +35,8 @@ def test_init(self):
                 )
         assert cls._config == {
             'foo': 'bar', 'baz': 2, 'regions': ['us-east-1'],
-            'account_id': '1234', 'cleanup_notify': ['foo@bar.com']
+            'account_id': '1234', 'cleanup_notify': ['foo@bar.com'],
+            'function_prefix': 'custodian-'
         }
         assert cls.config_path == 'manheim-c7n-tools.yml'
         assert mock_logger.mock_calls == [
@@ -59,11 +60,12 @@ def test_init_not_us_east_1(self):
                 cls = ManheimConfig(
                     foo='bar', baz=2, regions=['us-east-2'],
                     config_path='manheim-c7n-tools.yml', account_id='1234',
-                    cleanup_notify=['foo@bar.com']
+                    cleanup_notify=['foo@bar.com'], function_prefix='foo-'
                 )
         assert cls._config == {
             'foo': 'bar', 'baz': 2, 'regions': ['us-east-2'],
-            'account_id': '1234', 'cleanup_notify': ['foo@bar.com']
+            'account_id': '1234', 'cleanup_notify': ['foo@bar.com'],
+            'function_prefix': 'foo-'
         }
         assert cls.config_path == 'manheim-c7n-tools.yml'
         assert mock_logger.mock_calls == [
@@ -73,7 +75,8 @@ def test_init_not_us_east_1(self):
             call(
                 {
                     'foo': 'bar', 'baz': 2, 'regions': ['us-east-2'],
-                    'account_id': '1234', 'cleanup_notify': ['foo@bar.com']
+                    'account_id': '1234', 'cleanup_notify': ['foo@bar.com'],
+                    'function_prefix': 'foo-'
                 },
                 MANHEIM_CONFIG_SCHEMA
             )
@@ -260,7 +263,8 @@ def test_config_for_region(self):
             },
             'account_id': '012345',
             'regions': ['us-east-1', 'us-east-2'],
-            'cleanup_notify': []
+            'cleanup_notify': [],
+            'function_prefix': 'custodian-'
         }
         with patch('%s.jsonschema.validate' % pbm, autospec=True):
             with patch.dict(

manheim_c7n_tools/tests/test_policygen.py

@@ -74,6 +74,9 @@ def setup_method(self):
         type(self.m_conf).mailer_config = PropertyMock(
             return_value={'queue_url': 'MailerUrl'}
         )
+        type(self.m_conf).function_prefix = PropertyMock(
+            return_value='custodian-'
+        )
         type(self.m_conf).account_name = PropertyMock(return_value='myAccount')
         type(self.m_conf).account_id = PropertyMock(return_value='1234567890')
         self.m_conf.list_accounts.return_value = ['myAccount', 'otherAccount']
@@ -1905,6 +1908,66 @@ def se_strip_doc(func):
         ]
 
 
+class TestCheckPolicyFunctionPrefix(PolicyGenTester):
+
+    def test_success_no_prefix(self):
+        policy = {
+            'name': 'foo',
+            'mode': {
+                'type': 'periodic'
+            }
+        }
+        assert self.cls._check_policy_function_prefix(policy) is True
+
+    def test_success_default_prefix(self):
+        policy = {
+            'name': 'foo',
+            'mode': {
+                'type': 'periodic',
+                'function-prefix': 'custodian-'
+            }
+        }
+        assert self.cls._check_policy_function_prefix(policy) is True
+
+    def test_success_custom_prefix(self):
+        type(self.m_conf).function_prefix = PropertyMock(
+            return_value='foobar-'
+        )
+        policy = {
+            'name': 'foo',
+            'mode': {
+                'type': 'periodic',
+                'function-prefix': 'foobar-'
+            }
+        }
+        assert self.cls._check_policy_function_prefix(policy) is True
+
+    def test_fail_config_custom_prefix(self):
+        type(self.m_conf).function_prefix = PropertyMock(
+            return_value='foobar-'
+        )
+        policy = {
+            'name': 'foo',
+            'mode': {
+                'type': 'periodic'
+            }
+        }
+        assert self.cls._check_policy_function_prefix(policy) is False
+
+    def test_fail_policy_custom_prefix(self):
+        type(self.m_conf).function_prefix = PropertyMock(
+            return_value='custodian-'
+        )
+        policy = {
+            'name': 'foo',
+            'mode': {
+                'type': 'periodic',
+                'function-prefix': 'foobar-'
+            }
+        }
+        assert self.cls._check_policy_function_prefix(policy) is False
+
+
 class TestCheckPolicyMarkedForOpFirst(PolicyGenTester):
 
     def test_no_filters(self):

manheim_c7n_tools/tests/test_runner.py

@@ -123,6 +123,9 @@ def test_run_in_region(self):
 class TestMugcStep(StepTester):
 
     def test_run(self):
+        type(self.m_conf).function_prefix = PropertyMock(
+            return_value='foobar-'
+        )
         mock_pol1 = Mock(provider_name='foo')
         mock_pol2 = Mock(provider_name='aws')
         mock_pol3 = Mock(provider_name='azure')
@@ -149,8 +152,8 @@ def test_run(self):
             call(
                 config_files=['custodian_rName.yml'],
                 regions=['rName'],
-                prefix='custodian-',
-                policy_regex='^custodian-.*',
+                prefix='foobar-',
+                policy_regex='^foobar\\-.*',
                 assume=None,
                 policy_filter=None,
                 log_group=None,
@@ -175,6 +178,9 @@ def test_run(self):
         ]
 
     def test_dryrun(self):
+        type(self.m_conf).function_prefix = PropertyMock(
+            return_value='foobar-'
+        )
         mock_pol1 = Mock(provider_name='foo')
         mock_pol2 = Mock(provider_name='aws')
         mock_pol3 = Mock(provider_name='azure')
@@ -201,8 +207,8 @@ def test_dryrun(self):
             call(
                 config_files=['custodian_rName.yml'],
                 regions=['rName'],
-                prefix='custodian-',
-                policy_regex='^custodian-.*',
+                prefix='foobar-',
+                policy_regex='^foobar\\-.*',
                 assume=None,
                 policy_filter=None,
                 log_group=None,

manheim_c7n_tools/version.py

@@ -17,7 +17,7 @@
 """
 
 #: The semver-compliant version of the package.
-VERSION = '1.2.3'
+VERSION = '1.2.4'
 
 #: The URL for further information about the package.
 PROJECT_URL = 'https://github.com/manheim/manheim-c7n-tools'

setup.cfg

@@ -1,2 +1,6 @@
 [bdist_wheel]
 universal=1
+
+[pycodestyle]
+exclude = lib/*,lib64/*,manheim_c7n_tools/vendor/*
+max-line-length = 80

tox.ini

@@ -7,12 +7,12 @@ deps =
   cov-core
   coverage
   execnet
-  pep8
+  pycodestyle
   py
   pytest>=2.8.3
   pytest-cache
   pytest-cov
-  pytest-pep8
+  pytest-pycodestyle
   pytest-flakes
   pytest-html
   mock==3.0.5
@@ -35,7 +35,7 @@ commands =
     virtualenv --version
     pip --version
     pip freeze
-    py.test -rxs -vv --durations=10 --pep8 --flakes --blockage --cov-report term-missing --cov-report xml --cov-report html --cov-config {toxinidir}/.coveragerc --cov=manheim_c7n_tools --junitxml=testresults.xml --html=testresults.html {posargs} manheim_c7n_tools
+    py.test -rxs -vv --durations=10 --pycodestyle --flakes --blockage --cov-report term-missing --cov-report xml --cov-report html --cov-config {toxinidir}/.coveragerc --cov=manheim_c7n_tools --junitxml=testresults.xml --html=testresults.html {posargs} manheim_c7n_tools
 
 [testenv:docs]
 # this really just makes sure README.rst will parse on pypi