From 18668d24652b653cb2b562d4708b87ba99794b85 Mon Sep 17 00:00:00 2001 From: kpdev Date: Mon, 22 Feb 2021 08:29:43 +0100 Subject: [PATCH] Early validate for CIFS without security service. To create a share with protocol CIFS it is mandatory to add a security service to the share network beforehand. If this is forgotten the share ends up in error. Validate the security service association based on share protocol from given driver. Raise invalid request exception if needed association between share_network and security_service is missing for given driver e.g. ONTAP. DocImpact Closes-Bug: #1900752 Change-Id: Ib7e9850e6439ee5d04f826d129afb1ab06950ce7 --- manila/message/message_field.py | 7 ++ manila/share/driver.py | 1 + manila/share/drivers/dell_emc/driver.py | 3 + manila/share/drivers/dell_emc/plugins/base.py | 1 + .../drivers/dell_emc/plugins/isilon/isilon.py | 1 + .../dell_emc/plugins/powermax/connection.py | 4 ++ .../dell_emc/plugins/unity/connection.py | 4 ++ .../dell_emc/plugins/vnx/connection.py | 4 ++ .../dataontap/cluster_mode/drv_multi_svm.py | 4 ++ .../dataontap/cluster_mode/drv_single_svm.py | 1 + manila/share/manager.py | 30 ++++++++ .../share/drivers/dell_emc/test_driver.py | 1 + manila/tests/share/test_manager.py | 68 +++++++++++++++++++ ...-service-association-f48aecbbc47418cd.yaml | 8 +++ 14 files changed, 137 insertions(+) create mode 100644 releasenotes/notes/bug-1900752-early-validate-mandatory-security-service-association-f48aecbbc47418cd.yaml diff --git a/manila/message/message_field.py b/manila/message/message_field.py index ef6a36aa47..f4055d7ab5 100644 --- a/manila/message/message_field.py +++ b/manila/message/message_field.py @@ -131,6 +131,12 @@ class Detail(object): '024', _("No default share type has been made available. " "You must specify a share type for creating shares.")) + MISSING_SECURITY_SERVICE = ( + '025', + _("Share Driver failed to create share because a security service " + "has not been added to the share network used. Please add a " + "security service to the share network.")) + ALL = ( UNKNOWN_ERROR, NO_VALID_HOST, @@ -156,6 +162,7 @@ class Detail(object): UNSUPPORTED_ADD_UDPATE_SECURITY_SERVICE, SECURITY_SERVICE_FAILED_AUTH, NO_DEFAULT_SHARE_TYPE, + MISSING_SECURITY_SERVICE, ) # Exception and detail mappings diff --git a/manila/share/driver.py b/manila/share/driver.py index ef3b96d42a..37664b70dc 100644 --- a/manila/share/driver.py +++ b/manila/share/driver.py @@ -268,6 +268,7 @@ class ShareDriver(object): # in-use share networks. This property will be saved in every new share # server. self.security_service_update_support = False + self.dhss_mandatory_security_service_association = {} self.pools = [] if self.configuration: diff --git a/manila/share/drivers/dell_emc/driver.py b/manila/share/drivers/dell_emc/driver.py index 365874b1c0..9b95d6f2a3 100644 --- a/manila/share/drivers/dell_emc/driver.py +++ b/manila/share/drivers/dell_emc/driver.py @@ -87,6 +87,9 @@ class EMCShareDriver(driver.ShareDriver): super(EMCShareDriver, self).__init__( self.plugin.driver_handles_share_servers, *args, **kwargs) + self.dhss_mandatory_security_service_association = ( + self.plugin.dhss_mandatory_security_service_association) + if hasattr(self.plugin, 'ipv6_implemented'): self.ipv6_implemented = self.plugin.ipv6_implemented diff --git a/manila/share/drivers/dell_emc/plugins/base.py b/manila/share/drivers/dell_emc/plugins/base.py index 71c59fa0fd..1601e4d5bf 100644 --- a/manila/share/drivers/dell_emc/plugins/base.py +++ b/manila/share/drivers/dell_emc/plugins/base.py @@ -27,6 +27,7 @@ class StorageConnection(object): # NOTE(vponomaryov): redefine 'driver_handles_share_servers' within # plugin. self.driver_handles_share_servers = None + self.dhss_mandatory_security_service_association = {} @abc.abstractmethod def create_share(self, context, share, share_server): diff --git a/manila/share/drivers/dell_emc/plugins/isilon/isilon.py b/manila/share/drivers/dell_emc/plugins/isilon/isilon.py index 7ca5f1400d..8b20914440 100644 --- a/manila/share/drivers/dell_emc/plugins/isilon/isilon.py +++ b/manila/share/drivers/dell_emc/plugins/isilon/isilon.py @@ -55,6 +55,7 @@ class IsilonStorageConnection(base.StorageConnection): self._isilon_api = None self._isilon_api_class = isilon_api.IsilonApi self.driver_handles_share_servers = False + self.dhss_mandatory_security_service_association = {} def _get_container_path(self, share): """Return path to a container.""" diff --git a/manila/share/drivers/dell_emc/plugins/powermax/connection.py b/manila/share/drivers/dell_emc/plugins/powermax/connection.py index ae91806475..96a9932963 100644 --- a/manila/share/drivers/dell_emc/plugins/powermax/connection.py +++ b/manila/share/drivers/dell_emc/plugins/powermax/connection.py @@ -84,6 +84,10 @@ class PowerMaxStorageConnection(driver.StorageConnection): self.driver_handles_share_servers = True self.port_conf = None self.ipv6_implemented = True + self.dhss_mandatory_security_service_association = { + 'nfs': None, + 'cifs': 'active_directory', + } def create_share(self, context, share, share_server=None): """Create a share and export it based on protocol used.""" diff --git a/manila/share/drivers/dell_emc/plugins/unity/connection.py b/manila/share/drivers/dell_emc/plugins/unity/connection.py index 1004352a7c..cb92013415 100644 --- a/manila/share/drivers/dell_emc/plugins/unity/connection.py +++ b/manila/share/drivers/dell_emc/plugins/unity/connection.py @@ -107,6 +107,10 @@ class UnityStorageConnection(driver.StorageConnection): # props from super class. self.driver_handles_share_servers = (True, False) + self.dhss_mandatory_security_service_association = { + 'nfs': None, + 'cifs': 'active_directory', + } def connect(self, emc_share_driver, context): """Connect to Unity storage.""" diff --git a/manila/share/drivers/dell_emc/plugins/vnx/connection.py b/manila/share/drivers/dell_emc/plugins/vnx/connection.py index ba530ddd83..6f0e56ef33 100644 --- a/manila/share/drivers/dell_emc/plugins/vnx/connection.py +++ b/manila/share/drivers/dell_emc/plugins/vnx/connection.py @@ -81,6 +81,10 @@ class VNXStorageConnection(driver.StorageConnection): self.driver_handles_share_servers = True self.port_conf = None self.ipv6_implemented = True + self.dhss_mandatory_security_service_association = { + 'nfs': None, + 'cifs': 'active_directory', + } def create_share(self, context, share, share_server=None): """Create a share and export it based on protocol used.""" diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py index d0f495b197..7bb9b20e1a 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py @@ -38,6 +38,10 @@ class NetAppCmodeMultiSvmShareDriver(driver.ShareDriver): # NetApp driver supports updating security service for in use share # networks. self.security_service_update_support = True + self.dhss_mandatory_security_service_association = { + 'nfs': None, + 'cifs': ['active_directory', 'ldap', 'kerberos', ] + } def do_setup(self, context): self.library.do_setup(context) diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py index 5853fe3158..62e361ba63 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py @@ -35,6 +35,7 @@ class NetAppCmodeSingleSvmShareDriver(driver.ShareDriver): False, *args, **kwargs) self.library = lib_single_svm.NetAppCmodeSingleSVMFileStorageLibrary( self.DRIVER_NAME, **kwargs) + self.dhss_mandatory_security_service_association = {} def do_setup(self, context): self.library.do_setup(context) diff --git a/manila/share/manager.py b/manila/share/manager.py index 5755c9cfd2..f6f83aa862 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -2006,6 +2006,36 @@ class ShareManager(manager.SchedulerDependentManager): else: share_server = None + if share_network_id and self.driver.driver_handles_share_servers: + proto = share_instance.get('share_proto').lower() + ret_types = ( + self.driver.dhss_mandatory_security_service_association.get( + proto)) + if ret_types: + share_network = self.db.share_network_get(context, + share_network_id) + share_network_ss = [] + for security_service in share_network['security_services']: + share_network_ss.append(security_service['type'].lower()) + for types in ret_types: + if types not in share_network_ss: + self.db.share_instance_update( + context, share_instance_id, + {'status': constants.STATUS_ERROR} + ) + self.message_api.create( + context, + message_field.Action.CREATE, + share['project_id'], + resource_type=message_field.Resource.SHARE, + resource_id=share_id, + detail=(message_field.Detail + .MISSING_SECURITY_SERVICE)) + raise exception.InvalidRequest(_( + "Share network security service association is " + "mandatory for protocol %s.") % + share_instance.get('share_proto')) + status = constants.STATUS_AVAILABLE try: if snapshot_ref: diff --git a/manila/tests/share/drivers/dell_emc/test_driver.py b/manila/tests/share/drivers/dell_emc/test_driver.py index a9158ffafe..c95eb042f5 100644 --- a/manila/tests/share/drivers/dell_emc/test_driver.py +++ b/manila/tests/share/drivers/dell_emc/test_driver.py @@ -26,6 +26,7 @@ from manila import test class FakeConnection(base.StorageConnection): def __init__(self, *args, **kwargs): self.ipv6_implemented = True + self.dhss_mandatory_security_service_association = {} pass @property diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index f1a1da4356..81cf8484b5 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -2255,6 +2255,72 @@ class ShareManagerTestCase(test.TestCase): resource_id=shr['id'], detail=message_field.Detail.NO_SHARE_SERVER) + def test_create_share_instance_with_security_service_missing(self): + """Test creation fails if security service association is missing.""" + + self.mock_object(self.share_manager, 'driver') + self.share_manager.driver.driver_handles_share_servers = True + self.share_manager.driver.\ + dhss_mandatory_security_service_association = { + 'fake_proto': ['fake_ss', 'fake_ss2', ] + } + ss_data = { + 'name': 'fake_name', + 'ou': 'fake_ou', + 'domain': 'fake_domain', + 'server': 'fake_server', + 'dns_ip': 'fake_dns_ip', + 'user': 'fake_user', + 'type': 'fake_ss', + 'password': 'fake_pass', + } + security_service = db_utils.create_security_service(**ss_data) + share_net = db_utils.create_share_network() + share_net_subnet = db_utils.create_share_network_subnet( + share_network_id=share_net['id'], + availability_zone_id=None, + ) + db.share_network_add_security_service(context.get_admin_context(), + share_net['id'], + security_service['id']) + share = db_utils.create_share( + share_network_id=share_net['id'], + share_proto='fake_proto', + ) + db_utils.create_share_server( + share_network_subnet_id=share_net_subnet['id'], + host=self.share_manager.host, + status=constants.STATUS_ERROR) + fake_server = { + 'id': 'fake_srv_id', + 'status': constants.STATUS_CREATING, + } + fake_metadata = { + 'request_host': 'fake_host', + 'share_type_id': 'fake_share_type_id', + } + self.mock_object(self.share_manager, '_build_server_metadata', + mock.Mock(return_value=fake_metadata)) + self.mock_object(db, 'share_server_create', + mock.Mock(return_value=fake_server)) + self.mock_object(self.share_manager, '_setup_server', + mock.Mock(return_value=fake_server)) + self.assertRaises( + exception.InvalidRequest, + self.share_manager.create_share_instance, + self.context, + share.instance['id'] + ) + share = db.share_get(self.context, share['id']) + self.assertEqual(constants.STATUS_ERROR, share['status']) + self.share_manager.message_api.create.assert_called_once_with( + utils.IsAMatcher(context.RequestContext), + message_field.Action.CREATE, + str(share.project_id), + resource_type=message_field.Resource.SHARE, + resource_id=share['id'], + detail=message_field.Detail.MISSING_SECURITY_SERVICE) + @ddt.data( (True, 1, 3, 10, 0), (False, 1, 100, 5, 0), @@ -2419,6 +2485,8 @@ class ShareManagerTestCase(test.TestCase): share_srv ) self.share_manager.driver = driver_mock + self.share_manager.driver.\ + dhss_mandatory_security_service_association = {} self.share_manager.create_share_instance(self.context, share.instance['id']) self.assertFalse(self.share_manager.driver.setup_network.called) diff --git a/releasenotes/notes/bug-1900752-early-validate-mandatory-security-service-association-f48aecbbc47418cd.yaml b/releasenotes/notes/bug-1900752-early-validate-mandatory-security-service-association-f48aecbbc47418cd.yaml new file mode 100644 index 0000000000..7d587f03e4 --- /dev/null +++ b/releasenotes/notes/bug-1900752-early-validate-mandatory-security-service-association-f48aecbbc47418cd.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + For some drivers, to create a share with specific protocol it is mandatory + to add a security service to the share network beforehand. If this is + forgotten the share ends up in error. From now on, Manila won't allow + shares to be created when the specified protocol requires a specific + security service type that is not associated to the share network.