diff --git a/rest_api/rest_api_server/controllers/cloud_account.py b/rest_api/rest_api_server/controllers/cloud_account.py index 3c822b376..7b197dab8 100644 --- a/rest_api/rest_api_server/controllers/cloud_account.py +++ b/rest_api/rest_api_server/controllers/cloud_account.py @@ -536,18 +536,21 @@ def edit(self, item_id, **kwargs): if cloud_acc_obj.parent_id and config: raise WrongArgumentsException(Err.OE0211, ['config']) if cloud_acc_type in [CloudTypes.AWS_CNR.value]: - role_account_id = old_config.get('assume_role_account_id') - old_role = old_config.get('assume_role_name') - if role_account_id: - new_role_acc = config.get('assume_role_account_id') - if new_role_acc and (new_role_acc != role_account_id): - raise WrongArgumentsException(Err.OE0211, [ - 'assume_role_account_id']) + has_assumed_role_data = bool( + config.get('assume_role_account_id') or config.get('assume_role_name') + ) + # assumed + if has_assumed_role_data: + old_role_acc = old_config.get('assume_role_account_id') + role_acc = config.get('assume_role_account_id') or old_role_acc + + if old_role_acc and role_acc != old_role_acc: + raise WrongArgumentsException(Err.OE0211, ['assume_role_account_id']) + role = config.get('assume_role_name') - if new_role_acc: - if not role: - raise WrongArgumentsException(Err.OE0548, [ - 'assume_role_name']) + if not role: + raise WrongArgumentsException(Err.OE0548, [ + 'assume_role_name']) unexpected = ['access_key_id', 'secret_access_key'] for i in unexpected: if config.get(i): @@ -559,11 +562,10 @@ def edit(self, item_id, **kwargs): "access_key_id", "") config["secret_access_key"] = service_creds.get( "secret_access_key", "") - config["assume_role_account_id"] = role_account_id - config["assume_role_name"] = role or old_role + config["assume_role_account_id"] = role_acc + config["assume_role_name"] = role # non-assumed else: - access_key_id = config.get('access_key_id') secret_access_key = config.get('secret_access_key') if bool(access_key_id) ^ bool(secret_access_key): diff --git a/rest_api/rest_api_server/controllers/expense.py b/rest_api/rest_api_server/controllers/expense.py index d5a470644..4f481b91c 100644 --- a/rest_api/rest_api_server/controllers/expense.py +++ b/rest_api/rest_api_server/controllers/expense.py @@ -1746,16 +1746,20 @@ def get_expenses(self, cloud_account_ids, resource_ids, start_date, for n, filter_value in enumerate(filter_values): if filter_value == nil_uuid: filter_values[n] = None - filters = [ - {"cloud_account_id": {"$in": cloud_account_ids}}, - { - "$or": [ - {"resource_id": {"$in": cloud_resource_ids}}, - {"resource_hash": {"$in": cloud_resource_hashes}}, - ] - }, - ] - expenses = list(self.expense_ctrl.get_raw_expenses(start, end, filters)) + expenses = [] + for field, list_ids in [ + ('resource_id', cloud_resource_ids), + ('resource_hash', cloud_resource_hashes) + ]: + if not list_ids: + continue + filters = [ + {"cloud_account_id": {"$in": cloud_account_ids}}, + {field: {"$in": list_ids}}, + ] + expenses.extend( + list(self.expense_ctrl.get_raw_expenses(start, end, filters)) + ) return expenses, self.get_expenses_total_cost(expenses) def _get_cloud_resource_ids(self, resource_ids): diff --git a/rest_api/rest_api_server/tests/unittests/test_cloud_accounts.py b/rest_api/rest_api_server/tests/unittests/test_cloud_accounts.py index 5c13e1518..d5c769e62 100644 --- a/rest_api/rest_api_server/tests/unittests/test_cloud_accounts.py +++ b/rest_api/rest_api_server/tests/unittests/test_cloud_accounts.py @@ -189,29 +189,6 @@ def test_create_cloud_key_acc_key(self): self.assertEqual(code, 400) self.assertEqual(cloud_acc['error']['error_code'], 'OE0548') - def test_restrict_change_to_assumed(self): - valid_config = { - 'name': 'my cloud_acc', - 'type': 'aws_cnr', - 'config': { - 'access_key_id': 'key', - 'secret_access_key': 'secret', - } - } - code, cloud_acc = self.create_cloud_account(self.org_id, valid_config) - cloud_acc_id = cloud_acc["id"] - valid_config.pop('type') - # add valid parameter set for assumed account - # but restrict changed account type - for i in ['assume_role_account_id', 'assume_role_name']: - update_cfg = copy.deepcopy(valid_config) - update_cfg['config'][i] = '' - - code, cloud_acc = self.client.cloud_account_update( - cloud_acc_id, update_cfg) - self.assertEqual(code, 400) - self.assertEqual(cloud_acc['error']['error_code'], 'OE0212') - def test_create_aws_cloud_acc_assume(self): assume_role_account_id = '87629' assume_role_name = 'va-test' @@ -275,6 +252,76 @@ def test_edit_aws_cloud_acc_assume(self, t_schedule): self.assertEqual(new_cloud_acc['config']['assume_role_account_id'], config['config']['assume_role_account_id']) + @patch('rest_api.rest_api_server.controllers.cloud_account.' + 'ExpensesRecalculationScheduleController.schedule') + def test_edit_change_to_assumed(self, t_schedule): + valid_config = { + 'name': 'my cloud_acc', + 'type': 'aws_cnr', + 'config': { + 'access_key_id': 'key', + 'secret_access_key': 'secret', + } + } + code, cloud_acc = self.create_cloud_account(self.org_id, valid_config) + cloud_acc_id = cloud_acc["id"] + + # check service creds account id in config + patch('tools.cloud_adapter.clouds.aws.Aws.validate_credentials', + return_value={'account_id': cloud_acc_id, 'warnings': []}).start() + + # set valid parameters for an assumed account + role_acc_id = '87629' + role_acc = 'va-test' + assumed_config = { + 'config': { + 'assume_role_account_id': role_acc_id, + 'assume_role_name': role_acc, + } + } + code, cloud_acc = self.client.cloud_account_update(cloud_acc_id, assumed_config) + self.assertEqual(code, 200) + self.assertEqual(cloud_acc['config']['assume_role_account_id'], role_acc_id) + self.assertEqual(cloud_acc['config']['assume_role_name'], role_acc) + self.assertEqual( + cloud_acc['config']['access_key_id'], + self._aws_service_creds['access_key_id'], + ) + + @patch('rest_api.rest_api_server.controllers.cloud_account.' + 'ExpensesRecalculationScheduleController.schedule') + def test_edit_change_to_access_key(self, t_schedule): + config = { + 'name': 'assume_cloud_acc', + 'type': 'aws_cnr', + 'config': { + 'assume_role_account_id': '87629', + 'assume_role_name': 'va-test', + 'config_scheme': 'create_report' + } + } + + code, cloud_acc = self.create_cloud_account(self.org_id, config) + cloud_acc_id = cloud_acc["id"] + + # check service creds account id in config + patch('tools.cloud_adapter.clouds.aws.Aws.validate_credentials', + return_value={'account_id': cloud_acc_id, 'warnings': []}).start() + + # set valid parameters for an access key + key = 'key' + secret = 'secret' + access_config = { + 'config': { + 'access_key_id': key, + 'secret_access_key': secret, + } + } + + code, cloud_acc = self.client.cloud_account_update(cloud_acc_id, access_config) + self.assertEqual(code, 200) + self.assertDictEqual(cloud_acc['config'], {'access_key_id': key}) + @patch('rest_api.rest_api_server.controllers.cloud_account.' 'ExpensesRecalculationScheduleController.schedule') def test_edit_assume_restricted_keys(self, t_schedule):