Total refactor

Major commit to refactor lots of things:
 - Remove globals - config and dynamo object are passed explicitly
where required
 - Split Dynamo specific logic to a Dynamo utility and made it all
generic rather than specific Dynamo statements as functions
 - Wrapper for Dynamo calls that will catch and log errors
 - Lots of other stuff

.gitignore

@@ -1,4 +1,11 @@
 *.pyc
+*.json
+*.log
+*.csv
+build
+dist
+.coverage
+.cache
 .DS_Store
 .DS_Store?
 

.travis.yml

@@ -12,7 +12,7 @@ install:
 
 script:
   - coverage run --source repokid -m py.test
-  - bandit -r . -ll -ii -x tests/
+  - bandit -r . -ll -ii -x repokid/tests/
 
 after_success:
   - coveralls

repokid/__init__.py

@@ -15,7 +15,8 @@
 import logging.config
 import os
 
-__version__ = '0.6'
+__version__ = '0.7'
+
 
 def init_config():
     """

repokid/exceptions/__init__.py

@@ -0,0 +1,2 @@
+class UnexpectedDynamoUpdateValue(Exception):
+    pass

repokid/filters/age/__init__.py

@@ -1,8 +1,8 @@
 import datetime
 from dateutil.tz import tzlocal
 
-from repokid.repokid import Filter
-from repokid.repokid import LOGGER
+from repokid.cli.repokid_cli import Filter
+from repokid import LOGGER
 
 
 class AgeFilter(Filter):

repokid/filters/blacklist/__init__.py

@@ -1,5 +1,5 @@
-from repokid.repokid import Filter
-from repokid.repokid import LOGGER
+from repokid.cli.repokid_cli import Filter
+from repokid import LOGGER
 
 
 class BlacklistFilter(Filter):

repokid/filters/lambda/__init__.py

@@ -1,4 +1,4 @@
-from repokid.repokid import Filter
+from repokid.cli.repokid_cli import Filter
 
 
 class LambdaFilter(Filter):

repokid/filters/optout/__init__.py

@@ -1,6 +1,6 @@
 import time.time
 
-from repokid.repokid import Filter
+from repokid.cli.repokid_cli import Filter
 
 
 class OptOutFilter(Filter):

repokid/role.py

@@ -11,49 +11,39 @@
 #     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #     See the License for the specific language governing permissions and
 #     limitations under the License.
+import copy
+
+dict_to_attr = {'AAData': {'attribute': 'aa_data', 'default': dict()},
+                'Account': {'attribute': 'account', 'default': None},
+                'Active': {'attribute': 'active', 'default': True},
+                'Arn': {'attribute': 'arn', 'default': None},
+                'AssumeRolePolicyDocument': {'attribute': 'assume_role_policy_document', 'default': None},
+                'CreateDate': {'attribute': 'create_date', 'default': None},
+                'DisqualifiedBy': {'attribute': 'disqualified_by', 'default': list()},
+                'NoRepoPermissions': {'attribute': 'no_repo_permissions', 'default': dict()},
+                'OptOut': {'attribute': 'opt_out', 'default': dict()},
+                'Policies': {'attribute': 'policies', 'default': list()},
+                'Refreshed': {'attribute': 'refreshed', 'default': str()},
+                'RepoablePermissions': {'attribute': 'repoable_permissions', 'default': int()},
+                'RepoableServices': {'attribute': 'repoable_services', 'default': int()},
+                'Repoed': {'attribute': 'repoed', 'default': str()},
+                'RoleId': {'attribute': 'role_id', 'default': None},
+                'RoleName': {'attribute': 'role_name', 'default': None},
+                'Stats': {'attribute': 'stats', 'default': list()},
+                'TotalPermissions': {'attribute': 'total_permissions', 'default': int()}}
 
 
 class Role(object):
     def __init__(self, role_dict):
-        self.aa_data = role_dict.get('AAData', {})
-        self.active = role_dict.get('Active', True)
-        self.arn = role_dict.get('Arn', None)
-        self.assume_role_policy_document = role_dict.get('AssumeRolePolicyDocument', None)
-        self.create_date = role_dict.get('CreateDate', None)
-        self.disqualified_by = role_dict.get('DisqualifiedBy', [])
-        self.no_repo_permissions = role_dict.get('NoRepoPermissions', {})
-        self.opt_out = role_dict.get('OptOut', {})
-        self.policies = role_dict.get('Policies', [])
-        self.refreshed = role_dict.get('Refreshed', '')
-        self.repoable_permissions = role_dict.get('RepoablePermissions', 0)
-        self.repoable_services = role_dict.get('RepoableServices', [])
-        self.repoed = role_dict.get('Repoed', '')
-        self.role_id = role_dict.get('RoleId', None)
-        self.role_name = role_dict.get('RoleName', None)
-        self.stats = role_dict.get('Stats', [])
-        self.total_permissions = role_dict.get('TotalPermissions', 0)
-
-        self.account = role_dict.get('Account', None) or self.arn.split(':')[4] if self.arn else None
+        for key, value in dict_to_attr.items():
+            setattr(self, value['attribute'], role_dict[key] if key in role_dict else copy.copy(value['default']))
 
     def as_dict(self):
-        return {'AAData': self.aa_data,
-                'Account': self.account,
-                'Active': self.active,
-                'Arn': self.arn,
-                'AssumeRolePolicyDocument': self.assume_role_policy_document,
-                'CreateDate': self.create_date,
-                'DisqualifiedBy': self.disqualified_by,
-                'Policies': self.policies,
-                'NoRepoPermissions': self.no_repo_permissions,
-                'OptOut': self.opt_out,
-                'Refreshed': self.refreshed,
-                'RepoablePermissions': self.repoable_permissions,
-                'RepoableServices': self.repoable_services,
-                'Repoed': self.repoed,
-                'RoleId': self.role_id,
-                'RoleName': self.role_name,
-                'Stats': self.stats,
-                'TotalPermissions': self.total_permissions}
+        return {key: getattr(self, value['attribute']) for key, value in dict_to_attr.items()}
+
+    def set_attributes(self, attributes_dict):
+        for key, value in attributes_dict.items():
+            setattr(self, dict_to_attr[key]['attribute'], value)
 
     def __eq__(self, other):
         return self.role_id == other
@@ -76,14 +66,14 @@ def __len__(self):
         return len(self.roles)
 
     def __repr__(self):
-        return [role.role_id for role in self.roles]
+        return str([role.role_id for role in self.roles])
 
     def __eq__(self, other):
         return (all(role.role_id in other for role in self.roles) and
                 all(role.role_id in self.roles for role in other))
 
-    # def append(self, role):
-    #     self.roles.append(role)
+    def append(self, role):
+        self.roles.append(role)
 
     def role_id_list(self):
         return [role.role_id for role in self.roles]

tests/test_repokid.py → repokid/tests/test_repokid_cli.py

@@ -18,7 +18,7 @@
 
 from dateutil.tz import tzlocal
 
-import repokid.repokid
+import repokid.cli.repokid_cli
 from repokid.role import Role, Roles
 import repokid.utils.roledata
 
@@ -137,19 +137,17 @@
 ]
 
 
-class TestRepokid(object):
+class TestRepokidCLI(object):
     @patch('repokid.utils.roledata.update_stats')
-    @patch('repokid.utils.roledata.update_repoable_data')
-    @patch('repokid.utils.roledata.update_aardvark_data')
-    @patch('repokid.utils.roledata.update_filtered_roles')
     @patch('repokid.utils.roledata.find_and_mark_inactive')
     @patch('repokid.utils.roledata.update_role_data')
-    @patch('repokid.repokid._get_aardvark_data')
-    @patch('repokid.repokid.get_role_inline_policies')
-    @patch('repokid.repokid.list_roles')
+    @patch('repokid.cli.repokid_cli.set_role_data')
+    @patch('repokid.cli.repokid_cli._get_aardvark_data')
+    @patch('repokid.cli.repokid_cli.get_role_inline_policies')
+    @patch('repokid.cli.repokid_cli.list_roles')
     def test_repokid_update_role_cache(self, mock_list_roles, mock_get_role_inline_policies, mock_get_aardvark_data,
-                                       mock_update_role_data, mock_find_and_mark_inactive, mock_update_filtered_roles,
-                                       mock_update_aardvark_data, mock_update_repoable_data, mock_update_stats):
+                                       mock_set_role_data, mock_update_role_data, mock_find_and_mark_inactive,
+                                       mock_update_stats):
 
         # only active roles
         mock_list_roles.return_value = ROLES[:3]
@@ -160,51 +158,53 @@ def test_repokid_update_role_cache(self, mock_list_roles, mock_get_role_inline_p
 
         mock_get_aardvark_data.return_value = AARDVARK_DATA
 
-        def update_role_data(role, current_policies):
+        def update_role_data(dynamo_table, account_number, role, current_policies):
             role.policies = role.policies = [{'Policy': current_policies}]
 
         mock_update_role_data.side_effect = update_role_data
 
-        repokid.repokid.CONFIG = {"connection_iam": {},
-                                  "active_filters": ["repokid.filters.age:AgeFilter"],
-                                  "filter_config": {"AgeFilter": {"minimum_age": 90}}}
+        config = {"aardvark_api_location": "", "connection_iam": {},
+                  "active_filters": ["repokid.filters.age:AgeFilter"], "filter_config":
+                  {"AgeFilter": {"minimum_age": 90}, "BlacklistFilter": {}}}
 
         console_logger = logging.StreamHandler()
         console_logger.setLevel(logging.WARNING)
 
-        repokid.repokid.LOGGER = logging.getLogger('test')
-        repokid.repokid.LOGGER.addHandler(console_logger)
+        repokid.cli.repokid_cli.LOGGER = logging.getLogger('test')
+        repokid.cli.repokid_cli.LOGGER.addHandler(console_logger)
 
-        repokid.repokid.update_role_cache('123456789012')
+        dynamo_table = None
+        account_number = '123456789012'
+
+        repokid.cli.repokid_cli.update_role_cache(account_number, dynamo_table, config)
 
         # validate update data called for each role
-        assert mock_update_role_data.mock_calls == [call(Role(ROLES[0]), ROLE_POLICIES['all_services_used']),
-                                                    call(Role(ROLES[1]), ROLE_POLICIES['unused_ec2']),
-                                                    call(Role(ROLES[2]), ROLE_POLICIES['all_services_used'])]
+        assert mock_update_role_data.mock_calls == [
+            call(dynamo_table, account_number, Role(ROLES[0]), ROLE_POLICIES['all_services_used']),
+            call(dynamo_table, account_number, Role(ROLES[1]), ROLE_POLICIES['unused_ec2']),
+            call(dynamo_table, account_number, Role(ROLES[2]), ROLE_POLICIES['all_services_used'])]
 
         # all roles active
-        assert mock_find_and_mark_inactive.mock_calls == [call('123456789012',
+        assert mock_find_and_mark_inactive.mock_calls == [call(dynamo_table, account_number,
                                                           [Role(ROLES[0]), Role(ROLES[1]), Role(ROLES[2])])]
 
         roles = Roles([Role(ROLES[0]), Role(ROLES[1]), Role(ROLES[2])])
-        assert mock_update_filtered_roles.mock_calls == [call(roles)]
-
-        assert mock_update_aardvark_data.mock_calls == [call(AARDVARK_DATA, roles)]
 
         # TODO: validate total permission, repoable, etc are getting updated properly
-        assert mock_update_repoable_data.mock_calls == [call(roles)]
 
-        assert mock_update_stats.mock_calls == [call(roles, source='Scan')]
+        assert mock_update_stats.mock_calls == [call(dynamo_table, roles, source='Scan')]
+
+        # TODO: set_role_data called with
 
     @patch('tabview.view')
-    @patch('repokid.utils.roledata.get_role_data')
-    @patch('repokid.utils.roledata.role_ids_for_account')
+    @patch('repokid.cli.repokid_cli.get_role_data')
+    @patch('repokid.cli.repokid_cli.role_ids_for_account')
     def test_repokid_display_roles(self, mock_role_ids_for_account, mock_get_role_data, mock_tabview):
         console_logger = logging.StreamHandler()
         console_logger.setLevel(logging.WARNING)
 
-        repokid.repokid.LOGGER = logging.getLogger('test')
-        repokid.repokid.LOGGER.addHandler(console_logger)
+        repokid.cli.repokid_cli.LOGGER = logging.getLogger('test')
+        repokid.cli.repokid_cli.LOGGER.addHandler(console_logger)
 
         mock_role_ids_for_account.return_value = ['AROAABCDEFGHIJKLMNOPA', 'AROAABCDEFGHIJKLMNOPB',
                                                   'AROAABCDEFGHIJKLMNOPC', 'AROAABCDEFGHIJKLMNOPD']
@@ -217,8 +217,8 @@ def test_repokid_display_roles(self, mock_role_ids_for_account, mock_get_role_da
                                           ROLES_FOR_DISPLAY[3], ROLES_FOR_DISPLAY[0], ROLES_FOR_DISPLAY[1],
                                           ROLES_FOR_DISPLAY[2], ROLES_FOR_DISPLAY[3]]
 
-        repokid.repokid.display_roles('123456789012', inactive=True)
-        repokid.repokid.display_roles('123456789012', inactive=False)
+        repokid.cli.repokid_cli.display_roles('123456789012', None, inactive=True)
+        repokid.cli.repokid_cli.display_roles('123456789012', None, inactive=False)
 
         # first call has inactive role, second doesn't because it's filtered
         assert mock_tabview.mock_calls == [
@@ -236,7 +236,7 @@ def test_repokid_display_roles(self, mock_role_ids_for_account, mock_get_role_da
                   ['unused_ec2', "Someday", [], True, 4, 2, 'Never', ['ec2']]])]
 
     def test_generate_default_config(self):
-        generated_config = repokid.repokid._generate_default_config()
+        generated_config = repokid.cli.repokid_cli._generate_default_config()
 
         required_config_fields = ['filter_config', 'active_filters', 'aardvark_api_location', 'connection_iam',
                                   'dynamo_db', 'logging', 'repo_requirements']

tests/test_roledata.py → repokid/tests/test_roledata.py

@@ -125,7 +125,7 @@ def test_get_role_permissions(self, mock_all_permissions, mock_get_actions_from_
         assert permissions == set(ROLE_POLICIES['unused_ec2']['ec2_perms'])
 
     def test_get_repoable_permissions(self):
-        repokid.utils.roledata.CONFIG = {'filter_config': {'AgeFilter': {'minimum_age': 1}}}
+        minimum_age = 1
         repokid.utils.roledata.IAM_ACCESS_ADVISOR_UNSUPPORTED_SERVICES = ['service_2']
         repokid.utils.roledata.IAM_ACCESS_ADVISOR_UNSUPPORTED_ACTIONS = ['service_1:action_3', 'service_1:action_4']
 
@@ -141,7 +141,7 @@ def test_get_repoable_permissions(self):
         no_repo_permissions = {'service_4:action_1': time.time()-1, 'service_4:action_2': time.time()+1000}
 
         repoable_permissions = repokid.utils.roledata._get_repoable_permissions(permissions, aa_data,
-                                                                                no_repo_permissions)
+                                                                                no_repo_permissions, minimum_age)
         # service_1:action_3 and action_4 are unsupported actions, service_2 is an unsupported service, service_3
         # was used too recently, service_4 action 2 is in no_repo_permissions and not expired
         assert repoable_permissions == set(['service_1:action_1', 'service_1:action_2', 'service_4:action_1'])
@@ -170,7 +170,8 @@ def test_calculate_repo_scores(self, mock_get_repoable_permissions, mock_get_rol
 
         mock_get_repoable_permissions.side_effect = [set(['iam:AddRoleToInstanceProfile', 'iam:AttachRolePolicy'])]
 
-        repokid.utils.roledata._calculate_repo_scores(roles)
+        minimum_age = 90
+        repokid.utils.roledata._calculate_repo_scores(roles, minimum_age)
 
         assert roles[0].repoable_permissions == 2
         assert roles[0].repoable_services == ['iam']