From 385b4b77233be9ef13d90895412ef31cf6c5d328 Mon Sep 17 00:00:00 2001 From: st6218 Date: Thu, 9 Apr 2020 19:52:03 -0700 Subject: [PATCH] Additional changes to private flavors add_tenants logic: a) allow add tenant only if flavor already assigned to region(s) b) each tenant in the request will be validated against the regions assigned to flavor - at least ONE tenant must pass validation: i. if NO tenant in tenant list is associated with any of the regions assigned to the flavor, Ranger will reject the request entirely and user will be prompted to submit new request with valid tenants. ii. only the tenants in tenant list that pass validation will be kept in flavor tenant list; those that failed validation will be DROPPED from the tenant list. delete_region logic (only for flavors with tenants): a) if a tenant is associated only with the deleted region, the delete_region logic will delete the tenant from fms table and the tenant is dropped from the tenant list. However, if the tenant is associated with other regions still assigned to the flavor, the tenant stays in the list. Change-Id: I31935477733c8597741cf7c7c57350ab1e2b4452 --- .../fms_rest/data/sql_alchemy/data_manager.py | 11 ++ .../fms_rest/data/sql_alchemy/db_models.py | 7 + .../fms_rest/logic/flavor_logic.py | 165 +++++++++++++----- orm/tests/unit/fms/test_flavor_logic.py | 5 +- 4 files changed, 138 insertions(+), 50 deletions(-) diff --git a/orm/services/flavor_manager/fms_rest/data/sql_alchemy/data_manager.py b/orm/services/flavor_manager/fms_rest/data/sql_alchemy/data_manager.py index 4016c8d9..a568d074 100755 --- a/orm/services/flavor_manager/fms_rest/data/sql_alchemy/data_manager.py +++ b/orm/services/flavor_manager/fms_rest/data/sql_alchemy/data_manager.py @@ -111,3 +111,14 @@ class DataManager(object): results = datamanager.session.connection().execute(sql_query % (tenants_list, regions_list)).fetchall() return results + + def get_valid_tenant_list(self, requested_tenants): + # validate if tenants in list exist in customer table + + datamanager = DataManager() + # convert tenant and region lists to sql-friendly list format + tenants_list = str(tuple(requested_tenants)).rstrip(',)') + ')' + sql_query = '''select uuid from customer where uuid in %s''' + valid_tenants_list = datamanager.session.connection().execute(sql_query % (tenants_list)).fetchall() + + return valid_tenants_list diff --git a/orm/services/flavor_manager/fms_rest/data/sql_alchemy/db_models.py b/orm/services/flavor_manager/fms_rest/data/sql_alchemy/db_models.py index cfa663fa..d2e65398 100755 --- a/orm/services/flavor_manager/fms_rest/data/sql_alchemy/db_models.py +++ b/orm/services/flavor_manager/fms_rest/data/sql_alchemy/db_models.py @@ -346,6 +346,13 @@ class Flavor(Base, FMSBaseModel): return existing_region_names + def get_existing_tenant_ids(self): + existing_tenant_ids = [] + for tenant in self.flavor_tenants: + existing_tenant_ids.append(tenant.tenant_id) + + return existing_tenant_ids + ''' ' FlavorExtraSpec is a DataObject contains all fields defined in diff --git a/orm/services/flavor_manager/fms_rest/logic/flavor_logic.py b/orm/services/flavor_manager/fms_rest/logic/flavor_logic.py index f0e6c1c2..ae5c0439 100755 --- a/orm/services/flavor_manager/fms_rest/logic/flavor_logic.py +++ b/orm/services/flavor_manager/fms_rest/logic/flavor_logic.py @@ -14,26 +14,32 @@ from orm.common.orm_common.utils import utils from oslo_config import cfg import oslo_db + LOG = get_logger(__name__) di = injector.get_di() +def format_tenant_list(tenant_region_list): + # x[0] is the first element (tenant) in the tenant_region_list of tuples + # compile all the tenants and delete the NoneTypes from the tenant list + result_tenant_list = [x[0] for x in tenant_region_list if x[0]] + + # clean up valid_tenants_list by removing duplicate items + valid_tenants_list = list(dict.fromkeys(result_tenant_list)) + + return valid_tenants_list + + def validate_tenants_regions_list(requested_tenants, requested_regions, - action, datamanager): - regions_list, tenants_list = [], [] + datamanager): + valid_tenants_list, valid_regions_list = [], [] results = datamanager.get_valid_tenant_region_list(requested_tenants, requested_regions) - valid_tenants_list, valid_regions_list = [], [] - if results: - # the first element in the results tuple is the tenant prep - # result_tenant_list from result_rows and remove NoneTypes from list - result_tenant_list = [x[0] for x in results] - result_tenant_list = filter(None, result_tenant_list) - # lastly clean up valid_tenants_list list by removing duplicate items - valid_tenants_list = list(dict.fromkeys(result_tenant_list)) + # first element in the results tuple is the tenant + valid_tenants_list = format_tenant_list(results) # second element in the results tuple is the region - compile the # region data into valid_regions_list and eliminate duplicates @@ -63,18 +69,23 @@ def create_flavor(flavor, flavor_uuid, transaction_id): # Execute the following logic only if at least one region AND one # tenant are provided in a private flavor: - # Validate which tenants from the original tenants list to - # be assigned to the private flavor; if no valid tenants - # found, the valid_tenants_list will return empty list - if (flavor_regions and flavor.flavor.visibility == 'private' and - flavor.flavor.tenants): - valid_tenants_list, valid_regions_list = \ - validate_tenants_regions_list(flavor.flavor.tenants, - flavor_regions, - 'create', datamanager) + # identify which tenants from requested tenants list to be assigned + # to private flavor given the requested regions list; if no tenant + # identified, the valid_tenants_list will return empty list + if flavor_regions and flavor.flavor.visibility == 'private': + if flavor.flavor.tenants: + valid_tenants_list, valid_regions_list = \ + validate_tenants_regions_list(flavor.flavor.tenants, + flavor_regions, datamanager) + # replace the original tenant list in the private flavor # with the valid_tenants_list flavor.flavor.tenants = valid_tenants_list + else: + # disallow tenant assignment if no flavor region assigned + if flavor.flavor.tenants: + raise ErrorStatus( + 400, 'Cannot add tenants with no flavor region assigned ') sql_flavor = flavor.to_db_model() @@ -268,12 +279,17 @@ def add_regions(flavor_uuid, regions, transaction_id): # private flavor new logic # validate tenants assigned to the flavor for tenant in sql_flavor.flavor_tenants: - flvr_tenant_list.append(tenant.tenant_id) + flvr_tenant_list = sql_flavor.get_existing_tenant_ids() + + # existing_region_names = db_flavor.get_existing_region_names() + # add existing flavor regions to flvr_region_list in order for + # the validate_tenants_regions_list to process correctly + flvr_region_list = sql_flavor.get_existing_region_names() valid_tenants_list, valid_regions_list = \ validate_tenants_regions_list(flvr_tenant_list, - flvr_region_list, - 'create', datamanager) + flvr_region_list, datamanager) + # update tenant list for tenant in flvr_tenant_list: if tenant not in valid_tenants_list: @@ -289,7 +305,7 @@ def add_regions(flavor_uuid, regions, transaction_id): return ret except ErrorStatus as exp: - LOG.log_exception("FlavorLogic - Failed to update flavor", str(exp)) + LOG.log_exception("FlavorLogic - Failed to add regions", str(exp)) datamanager.rollback() raise exp except Exception as exp: @@ -303,11 +319,52 @@ def add_regions(flavor_uuid, regions, transaction_id): datamanager.close() +def delete_orphaned_tenants(sql_flavor, remaining_regions, datamanager): + # this function deletes all 'orphaned tenants' - i.e., tenants that + # are no longer associated with regions still assigned to the flavor + + try: + # get existing tenants_list and orig_regions_list BEFORE remove_region + existing_tenants_list = sql_flavor.get_existing_tenant_ids() + + valid_tenants_list, valid_regions_list = [], [] + # use 'remaining_regions' to identify the valid tenants list + # for all regions assigned to flavor MINUS the deleted region + if remaining_regions and existing_tenants_list: + valid_tenants_list, valid_regions_list = \ + validate_tenants_regions_list(existing_tenants_list, + remaining_regions, + datamanager) + + # remove orphaned tenants identified + if valid_tenants_list: + for tenant in existing_tenants_list: + if tenant not in valid_tenants_list: + sql_flavor.remove_tenant(tenant) + else: + if existing_tenants_list: + for tenant in existing_tenants_list: + sql_flavor.remove_tenant(tenant) + + datamanager.flush() # get any exception from remove_tenant action + datamanager.commit() + + except ErrorStatus as exp: + LOG.log_exception("FlavorLogic - Failed to remove tenant", str(exp)) + datamanager.rollback() + raise exp + + except Exception as exp: + LOG.log_exception( + "FlavorLogic - Failed to remove tenant - exception:", str(exp)) + datamanager.rollback() + raise exp + + @di.dependsOn('data_manager') def delete_region(flavor_uuid, region_name, transaction_id, force_delete): DataManager = di.resolver.unpack(delete_region) datamanager = DataManager() - try: flavor_rec = datamanager.get_record('flavor') sql_flavor = flavor_rec.get_flavor_by_id(flavor_uuid) @@ -317,26 +374,33 @@ def delete_region(flavor_uuid, region_name, transaction_id, force_delete): existing_region_names = sql_flavor.get_existing_region_names() sql_flavor.remove_region(region_name) + remaining_regions = sql_flavor.get_existing_region_names() # get any exception created by previous actions against the database datamanager.flush() + send_to_rds_if_needed(sql_flavor, existing_region_names, "put", transaction_id) + if force_delete: datamanager.commit() else: datamanager.rollback() except ErrorStatus as exp: - LOG.log_exception("FlavorLogic - Failed to update flavor", str(exp)) + LOG.log_exception("FlavorLogic - Failed to delete region", str(exp)) datamanager.rollback() raise exp except Exception as exp: - LOG.log_exception("FlavorLogic - Failed to delete region", str(exp)) + LOG.log_exception( + "FlavorLogic - Failed to delete region - exception:", str(exp)) datamanager.rollback() raise exp + else: + delete_orphaned_tenants(sql_flavor, remaining_regions, datamanager) + finally: datamanager.close() @@ -345,9 +409,6 @@ def delete_region(flavor_uuid, region_name, transaction_id, force_delete): def add_tenants(flavor_uuid, tenants, transaction_id): DataManager = di.resolver.unpack(add_tenants) datamanager = DataManager() - - valid_tenants_list, valid_regions_list = [], [] - try: flavor_rec = datamanager.get_record('flavor') sql_flavor = flavor_rec.get_flavor_by_id(flavor_uuid) @@ -358,32 +419,42 @@ def add_tenants(flavor_uuid, tenants, transaction_id): if sql_flavor.visibility == "public": raise ErrorStatus(405, 'Cannot add tenant to a public flavor') - existing_region_names = sql_flavor.get_existing_region_names() - existing_region_list = [] + existing_region_list = sql_flavor.get_existing_region_names() - for x in existing_region_names: - existing_region_list.append(x) + # remove any empty strings in tenants list + while ('' in tenants.tenants): + tenants.tenants.remove('') if tenants.tenants: - valid_tenants_list, valid_regions_list = \ - validate_tenants_regions_list(tenants.tenants, - existing_region_list, - 'create', datamanager) + for tenant in tenants.tenants: + if not isinstance(tenant, str): + raise ValueError("tenant type must be a string type," + " got {} type".format(type(tenant))) - if not valid_tenants_list: + if existing_region_list: + # get lists of valid tenants/regions from CMS + valid_tenants_list, valid_regions_list = \ + validate_tenants_regions_list(tenants.tenants, + existing_region_list, + datamanager) + else: + # disallow tenant assignment if no flavor region assigned + raise ErrorStatus( + 400, 'Cannot add tenants with no flavor region assigned') + + if not (tenants.tenants and valid_tenants_list): raise ValueError("At least one valid tenant must be provided") for tenant in valid_tenants_list: - if not isinstance(tenant, str): - raise ValueError("tenant type must be a string type," - " got {} type".format(type(tenant))) - db_tenant = FlavorTenant(tenant_id=tenant) sql_flavor.add_tenant(db_tenant) + + send_to_rds_if_needed( + sql_flavor, existing_region_list, "put", transaction_id) + # get any exception created by previous actions against the database datamanager.flush() - send_to_rds_if_needed( - sql_flavor, existing_region_names, "put", transaction_id) + datamanager.commit() flavor = get_flavor_by_uuid(flavor_uuid) @@ -391,7 +462,7 @@ def add_tenants(flavor_uuid, tenants, transaction_id): return ret except (ErrorStatus, ValueError) as exp: - LOG.log_exception("FlavorLogic - Failed to update flavor", str(exp)) + LOG.log_exception("FlavorLogic - Failed to add tenants", str(exp)) datamanager.rollback() raise except Exception as exp: @@ -408,14 +479,13 @@ def add_tenants(flavor_uuid, tenants, transaction_id): def delete_tenant(flavor_uuid, tenant_id, transaction_id): DataManager = di.resolver.unpack(delete_tenant) datamanager = DataManager() - try: flavor_rec = datamanager.get_record('flavor') sql_flavor = flavor_rec.get_flavor_by_id(flavor_uuid) if not sql_flavor: raise ErrorStatus(404, 'flavor id {0} not found'.format(flavor_uuid)) - # if trying to delete the only one tenant then return value error + if sql_flavor.visibility == "public": raise ValueError("{} is a public flavor, delete tenant" " action is not relevant".format(flavor_uuid)) @@ -756,7 +826,6 @@ def get_flavor_by_uuid(flavor_uuid): datamanager = DataManager() try: flavor_record = datamanager.get_record('flavor') - sql_flavor = flavor_record.get_flavor_by_id(flavor_uuid) if not sql_flavor: diff --git a/orm/tests/unit/fms/test_flavor_logic.py b/orm/tests/unit/fms/test_flavor_logic.py index 5ebc7d0c..f24682b2 100755 --- a/orm/tests/unit/fms/test_flavor_logic.py +++ b/orm/tests/unit/fms/test_flavor_logic.py @@ -508,10 +508,11 @@ class TestFlavorLogic(FunctionalTest): ret_flavor = MagicMock() tenants = ['test_tenant'] ret_flavor.flavor.tenants = tenants + ret_flavor.flavor.regions = [Region(name='test_region')] mock_gfbu.return_value = ret_flavor mock_val.return_value = ['test_tenant'], ['test_region'] global error - error = 31 + error = 3 injector.override_injected_dependency( ('rds_proxy', get_rds_proxy_mock())) @@ -546,7 +547,6 @@ class TestFlavorLogic(FunctionalTest): TenantWrapper(tenants), 'transaction') - # test that no valid tenant found error = 31 mock_val.return_value = [], ['test_region'] self.assertRaises(ValueError, @@ -554,6 +554,7 @@ class TestFlavorLogic(FunctionalTest): TenantWrapper(tenants), 'transaction') + error = 3 mock_val.return_value = ['test_tenant'], ['test_region'] mock_strin.side_effect = exc.FlushError( 'conflicts with persistent instance')