From da55238161f023a2ad13d5685a4e2c4809d0775a Mon Sep 17 00:00:00 2001 From: Travis McPeak Date: Thu, 5 Oct 2017 11:30:07 -0700 Subject: [PATCH] Fix a bug in opt-out filter This commit fixes a bug in opt-out filter caused by failing to get the current opt-out status from the Dynamo table. Now, as part of getting an existing role we'll update all data from Dynamo. Also force an update_role_cache prior to repo_all_roles as a safety mechanism. --- repokid/cli/repokid_cli.py | 12 +++++++++--- repokid/tests/test_repokid_cli.py | 12 ++++++------ repokid/utils/roledata.py | 5 ++++- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/repokid/cli/repokid_cli.py b/repokid/cli/repokid_cli.py index 1f50427ad..cf23a0686 100644 --- a/repokid/cli/repokid_cli.py +++ b/repokid/cli/repokid_cli.py @@ -845,7 +845,7 @@ def rollback_role(account_number, role_name, dynamo_table, config, hooks, select return errors -def repo_all_roles(account_number, dynamo_table, config, commit=False, scheduled=True): +def repo_all_roles(account_number, dynamo_table, config, hooks, commit=False, scheduled=True): """ Repo all scheduled or eligible roles in an account. Collect any errors and display them at the end. @@ -877,7 +877,7 @@ def repo_all_roles(account_number, dynamo_table, config, commit=False, scheduled ', '.join([role.role_name for role in roles]))) for role in roles: - error = repo_role(account_number, role.role_name, dynamo_table, config, commit=commit) + error = repo_role(account_number, role.role_name, dynamo_table, config, hooks, commit=commit) if error: errors.append(error) @@ -969,10 +969,16 @@ def main(): return rollback_role(account_number, role_name, dynamo_table, config, hooks, selection=selection, commit=commit) if args.get('repo_all_roles'): + LOGGER.info('Updating role data') + update_role_cache(account_number, dynamo_table, config, hooks) + LOGGER.info('Repoing all roles') commit = args.get('--commit') - return repo_all_roles(account_number, dynamo_table, config, commit=commit, scheduled=False) + return repo_all_roles(account_number, dynamo_table, config, hooks, commit=commit, scheduled=False) if args.get('repo_scheduled_roles'): + LOGGER.info('Updating role data') + update_role_cache(account_number, dynamo_table, config, hooks) + LOGGER.info('Repoing scheduled roles') commit = args.get('--commit') return repo_all_roles(account_number, dynamo_table, config, commit=commit, scheduled=True) diff --git a/repokid/tests/test_repokid_cli.py b/repokid/tests/test_repokid_cli.py index bc3405a26..af48fc463 100644 --- a/repokid/tests/test_repokid_cli.py +++ b/repokid/tests/test_repokid_cli.py @@ -282,14 +282,14 @@ def test_repo_all_roles(self, mock_time, mock_repo_role, mock_role_ids_for_accou mock_repo_role.return_value = None # repo all roles in the account, should call repo with all roles - repokid.cli.repokid_cli.repo_all_roles(None, None, None, scheduled=False) + repokid.cli.repokid_cli.repo_all_roles(None, None, None, None, scheduled=False) # repo only scheduled, should only call repo role with role A - repokid.cli.repokid_cli.repo_all_roles(None, None, None, scheduled=True) + repokid.cli.repokid_cli.repo_all_roles(None, None, None, None, scheduled=True) - assert mock_repo_role.mock_calls == [call(None, 'ROLE_A', None, None, commit=False), - call(None, 'ROLE_B', None, None, commit=False), - call(None, 'ROLE_C', None, None, commit=False), - call(None, 'ROLE_C', None, None, commit=False)] + assert mock_repo_role.mock_calls == [call(None, 'ROLE_A', None, None, None, commit=False), + call(None, 'ROLE_B', None, None, None, commit=False), + call(None, 'ROLE_C', None, None, None, commit=False), + call(None, 'ROLE_C', None, None, None, commit=False)] def test_generate_default_config(self): generated_config = repokid.cli.repokid_cli._generate_default_config() diff --git a/repokid/utils/roledata.py b/repokid/utils/roledata.py index a9199cb68..2ce1c9e36 100644 --- a/repokid/utils/roledata.py +++ b/repokid/utils/roledata.py @@ -181,7 +181,10 @@ def update_role_data(dynamo_table, account_number, role, current_policy, source= update_opt_out(dynamo_table, role) set_role_data(dynamo_table, role.role_id, {'Refreshed': datetime.datetime.utcnow().isoformat()}) - role.policies = get_role_data(dynamo_table, role.role_id, fields=['Policies'])['Policies'] + # Update all data from Dynamo except CreateDate (it's in the wrong format) + current_role_data = get_role_data(dynamo_table, role.role_id) + current_role_data.pop('CreateDate') + role.set_attributes(current_role_data) def update_stats(dynamo_table, roles, source='Scan'):