From 733d6218e6a569843e455bcd350c86c8a183ce42 Mon Sep 17 00:00:00 2001 From: Eduardo Santos Date: Wed, 10 Feb 2021 21:21:39 -0300 Subject: [PATCH] Add security service update support to the container driver This implementation adds the functionality to add/update security services to in use share networks using the container driver. The container driver will also try to setup security services while creating share servers. Currently, the only supported security service type is LDAP. Co-Authored-By: Carlos Eduardo Partially Implements: bp add-security-service-in-use-share-networks Depends-On: I129a794dfd2d179fa2b9a2fed050459d6f00b0de Change-Id: Ifb8b9ebe6eb0661844c794ca1a32e35105652f72 --- manila/share/drivers/container/driver.py | 109 ++++++++++- .../container/security_service_helper.py | 107 +++++++++++ .../share/drivers/container/test_driver.py | 107 +++++++++++ .../container/test_security_service_helper.py | 173 ++++++++++++++++++ ...-on-container-driver-52193447c18e6d10.yaml | 7 + 5 files changed, 502 insertions(+), 1 deletion(-) create mode 100644 manila/share/drivers/container/security_service_helper.py create mode 100644 manila/tests/share/drivers/container/test_security_service_helper.py create mode 100644 releasenotes/notes/add-and-update-security-services-to-in-use-share-servers-on-container-driver-52193447c18e6d10.yaml diff --git a/manila/share/drivers/container/driver.py b/manila/share/drivers/container/driver.py index 4daf3dc3ec..8a500b269c 100644 --- a/manila/share/drivers/container/driver.py +++ b/manila/share/drivers/container/driver.py @@ -64,6 +64,11 @@ container_opts = [ default="manila.share.drivers.container.protocol_helper." "DockerCIFSHelper", help="Helper which facilitates interaction with share server."), + cfg.StrOpt("container_security_service_helper", + default="manila.share.drivers.container.security_service_helper" + ".SecurityServiceHelper", + help="Helper which facilitates interaction with security " + "services."), cfg.StrOpt("container_storage_helper", default="manila.share.drivers.container.storage_helper." "LVMHelper", @@ -87,6 +92,9 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): self.container = importutils.import_class( self.configuration.container_helper)( configuration=self.configuration) + self.security_service_helper = importutils.import_class( + self.configuration.container_security_service_helper)( + configuration=self.configuration) self.storage = importutils.import_class( self.configuration.container_storage_helper)( configuration=self.configuration) @@ -118,7 +126,8 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): 'snapshot_support': False, 'create_share_from_snapshot_support': False, 'driver_name': 'ContainerShareDriver', - 'pools': self.storage.get_share_server_pools() + 'pools': self.storage.get_share_server_pools(), + 'security_service_update_support': True } super(ContainerShareDriver, self)._update_share_stats(data) @@ -294,6 +303,11 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): except Exception as e: raise exception.ManilaException(_("Cannot create container: %s") % e) + security_services = network_info.get('security_services') + + if security_services: + self.setup_security_services(server_id, security_services) + veths_after = self._get_veth_state() veth = self._get_corresponding_veth(veths_before, veths_after) @@ -549,3 +563,96 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): return { 'share_updates': shares_updates, } + + def setup_security_services(self, share_server_id, security_services): + """Is called to setup a security service in the share server.""" + + for security_service in security_services: + if security_service['type'].lower() != 'ldap': + raise exception.ShareBackendException(_( + "The container driver does not support security services " + "other than LDAP.")) + + self.security_service_helper.setup_security_service( + share_server_id, security_service) + + def _get_different_security_service_keys( + self, current_security_service, new_security_service): + valid_keys = ['dns_ip', 'server', 'domain', 'user', 'password', 'ou'] + different_keys = [] + for key, value in current_security_service.items(): + if (current_security_service[key] != new_security_service[key] + and key in valid_keys): + different_keys.append(key) + return different_keys + + def _check_if_all_fields_are_updatable(self, current_security_service, + new_security_service): + # NOTE(carloss): We only support updating user and password at + # the moment + updatable_fields = ['user', 'password'] + different_keys = self._get_different_security_service_keys( + current_security_service, new_security_service) + for key in different_keys: + if key not in updatable_fields: + return False + return True + + def update_share_server_security_service(self, context, share_server, + network_info, + share_instances, + share_instance_rules, + new_security_service, + current_security_service=None): + """Is called to update or add a sec service to a share server.""" + + if not self.check_update_share_server_security_service( + context, share_server, network_info, share_instances, + share_instance_rules, new_security_service, + current_security_service=current_security_service): + raise exception.ManilaException(_( + "The requested security service update is not supported by " + "the container driver.")) + + server_id = self._get_container_name(share_server['id']) + + if not current_security_service: + self.setup_security_services(server_id, [new_security_service]) + else: + self.security_service_helper.update_security_service( + server_id, current_security_service, new_security_service) + + msg = ( + "The security service was successfully added to the share " + "server %(server_id)s.") + msg_args = { + 'server_id': share_server['id'], + } + LOG.info(msg, msg_args) + + def check_update_share_server_security_service( + self, context, share_server, network_info, share_instances, + share_instance_rules, new_security_service, + current_security_service=None): + current_type = ( + current_security_service['type'].lower() + if current_security_service else '') + new_type = new_security_service['type'].lower() + + if new_type != 'ldap' or (current_type and current_type != 'ldap'): + LOG.error('Currently only LDAP security services are supported ' + 'by the container driver.') + return False + + if not current_type: + return True + + all_fields_are_updatable = self._check_if_all_fields_are_updatable( + current_security_service, new_security_service) + if not all_fields_are_updatable: + LOG.info( + "The Container driver does not support updating " + "security service parameters other than 'user' and " + "'password'.") + return False + return True diff --git a/manila/share/drivers/container/security_service_helper.py b/manila/share/drivers/container/security_service_helper.py new file mode 100644 index 0000000000..d52095ece3 --- /dev/null +++ b/manila/share/drivers/container/security_service_helper.py @@ -0,0 +1,107 @@ +# Copyright (c) 2021 NetApp, Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, 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. + + +from oslo_log import log as logging + +from manila import exception +from manila.i18n import _ +from manila.share import driver +from manila import utils as manila_utils + +# LDAP error codes +LDAP_INVALID_CREDENTIALS = 49 + +LOG = logging.getLogger(__name__) + + +class SecurityServiceHelper(driver.ExecuteMixin): + def __init__(self, *args, **kwargs): + self.configuration = kwargs.pop("configuration", None) + super(SecurityServiceHelper, self).__init__(*args, **kwargs) + self.init_execute_mixin() + + def setup_security_service(self, share_server_id, security_service): + msg = ("Setting up the security service %(service)s for share server " + "%(server_id)s") + msg_args = { + 'service': security_service['id'], + 'server_id': share_server_id + } + LOG.debug(msg, msg_args) + self.ldap_bind(share_server_id, security_service) + + def update_security_service(self, server_id, current_security_service, + new_security_service): + msg = ("Updating the security service %(service)s for share server " + "%(server_id)s") + msg_args = { + 'service': new_security_service['id'], + 'server_id': server_id + } + LOG.debug(msg, msg_args) + self.ldap_bind(server_id, new_security_service) + + def ldap_bind(self, share_server_id, security_service): + ss_info = self.ldap_get_info(security_service) + cmd = ["docker", "exec", "%s" % share_server_id, "ldapwhoami", "-x", + "-H", "ldap://localhost:389", "-D", + "cn=%s,dc=example,dc=com" % ss_info["ss_user"], "-w", "%s" % + ss_info["ss_password"]] + self.ldap_retry_operation(cmd, run_as_root=True) + + def ldap_get_info(self, security_service): + if all(info in security_service for info in ("user", "password")): + ss_user = security_service["user"] + ss_password = security_service["password"] + else: + raise exception.ShareBackendException( + _("LDAP requires user and password to be set for the bind " + "operation.")) + ss_info = { + "ss_user": ss_user, + "ss_password": ss_password, + } + return ss_info + + def ldap_retry_operation(self, cmd, run_as_root=True, timeout=30): + interval = 5 + retries = int(timeout / interval) or 1 + + @manila_utils.retry(exception.ProcessExecutionError, interval=interval, + retries=retries, backoff_rate=1) + def try_ldap_operation(): + try: + self._execute(*cmd, run_as_root=run_as_root) + except exception.ProcessExecutionError as e: + if e.exit_code == LDAP_INVALID_CREDENTIALS: + msg = _('LDAP credentials are invalid. ' + 'Aborting operation.') + LOG.warning(msg) + raise exception.ShareBackendException(msg=msg) + else: + msg = _('Command has returned execution error.' + ' Will retry the operation.' + ' Error details: %s') % e.stderr + LOG.warning(msg) + raise exception.ProcessExecutionError() + + try: + try_ldap_operation() + except exception.ProcessExecutionError as e: + msg = _("Unable to execute LDAP operation with success. " + "Retries exhausted. Error details: %s") % e.stderr + LOG.exception(msg) + raise exception.ShareBackendException(msg=msg) diff --git a/manila/tests/share/drivers/container/test_driver.py b/manila/tests/share/drivers/container/test_driver.py index b3e413b6ea..04bb5b066f 100644 --- a/manila/tests/share/drivers/container/test_driver.py +++ b/manila/tests/share/drivers/container/test_driver.py @@ -27,6 +27,7 @@ from manila.share import configuration from manila.share.drivers.container import driver from manila.share.drivers.container import protocol_helper from manila import test +from manila.tests import db_utils from manila.tests import fake_utils from manila.tests.share.drivers.container import fakes as cont_fakes @@ -608,3 +609,109 @@ class ContainerShareDriverTestCase(test.TestCase): mock_get_container_name.assert_any_call(source_server['id']) mock_get_container_name.assert_any_call(dest_server['id']) + + def test__get_different_security_service_keys(self): + sec_service_keys = ['dns_ip', 'server', 'domain', 'user', 'password', + 'ou'] + current_security_service = {} + [current_security_service.update({key: key + '_1'}) + for key in sec_service_keys] + new_security_service = {} + [new_security_service.update({key: key + '_2'}) + for key in sec_service_keys] + + db_utils.create_security_service(**current_security_service) + db_utils.create_security_service(**new_security_service) + + different_keys = self._driver._get_different_security_service_keys( + current_security_service, new_security_service) + + [self.assertIn(key, different_keys) for key in sec_service_keys] + + @ddt.data( + (['dns_ip', 'server', 'domain', 'user', 'password', 'ou'], False), + (['user', 'password'], True) + ) + @ddt.unpack + def test__check_if_all_fields_are_updatable(self, keys, expected_result): + + current_security_service = db_utils.create_security_service() + new_security_service = db_utils.create_security_service() + + mock_get_keys = self.mock_object( + self._driver, '_get_different_security_service_keys', + mock.Mock(return_value=keys)) + + result = self._driver._check_if_all_fields_are_updatable( + current_security_service, new_security_service) + + self.assertEqual(expected_result, result) + mock_get_keys.assert_called_once_with( + current_security_service, new_security_service + ) + + @ddt.data(True, False) + def test_update_share_server_security_service( + self, with_current_service): + new_security_service = db_utils.create_security_service() + current_security_service = ( + db_utils.create_security_service() + if with_current_service else None) + share_server = db_utils.create_share_server() + fake_container_name = 'fake_name' + network_info = {} + share_instances = [] + share_instance_access_rules = [] + + mock_check_update = self.mock_object( + self._driver, 'check_update_share_server_security_service', + mock.Mock(return_value=True)) + mock_get_container_name = self.mock_object( + self._driver, '_get_container_name', + mock.Mock(return_value=fake_container_name)) + mock_setup = self.mock_object(self._driver, 'setup_security_services') + mock_update_sec_service = self.mock_object( + self._driver.security_service_helper, 'update_security_service') + + self._driver.update_share_server_security_service( + self._context, share_server, network_info, share_instances, + share_instance_access_rules, new_security_service, + current_security_service=current_security_service) + + mock_check_update.assert_called_once_with( + self._context, share_server, network_info, share_instances, + share_instance_access_rules, new_security_service, + current_security_service=current_security_service + ) + mock_get_container_name.assert_called_once_with(share_server['id']) + if with_current_service: + mock_update_sec_service.assert_called_once_with( + fake_container_name, current_security_service, + new_security_service) + else: + mock_setup.assert_called_once_with( + fake_container_name, [new_security_service]) + + def test_update_share_server_security_service_not_supported(self): + new_security_service = db_utils.create_security_service() + current_security_service = db_utils.create_security_service() + share_server = db_utils.create_share_server() + share_instances = [] + share_instance_access_rules = [] + network_info = {} + + mock_check_update = self.mock_object( + self._driver, 'check_update_share_server_security_service', + mock.Mock(return_value=False)) + + self.assertRaises( + exception.ManilaException, + self._driver.update_share_server_security_service, + self._context, share_server, network_info, share_instances, + share_instance_access_rules, new_security_service, + current_security_service=current_security_service) + + mock_check_update.assert_called_once_with( + self._context, share_server, network_info, share_instances, + share_instance_access_rules, new_security_service, + current_security_service=current_security_service) diff --git a/manila/tests/share/drivers/container/test_security_service_helper.py b/manila/tests/share/drivers/container/test_security_service_helper.py new file mode 100644 index 0000000000..e17aee0e62 --- /dev/null +++ b/manila/tests/share/drivers/container/test_security_service_helper.py @@ -0,0 +1,173 @@ +# Copyright (c) 2021 NetApp, Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, 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. +"""Unit tests for the Security Service helper module.""" + +from unittest import mock + +import ddt + +from manila import exception +from manila.share import configuration +from manila.share.drivers.container import security_service_helper +from manila import test +from manila.tests import db_utils + + +INVALID_CREDENTIALS_EXIT_CODE = 49 + + +@ddt.ddt +class SecurityServiceHelperTestCase(test.TestCase): + """Tests DockerExecHelper""" + + def setUp(self): + super(SecurityServiceHelperTestCase, self).setUp() + self.fake_conf = configuration.Configuration(None) + self.fake_conf.container_image_name = "fake_image" + self.fake_conf.container_volume_mount_path = "/tmp/shares" + self.security_service_helper = ( + security_service_helper.SecurityServiceHelper( + configuration=self.fake_conf)) + + def test_setup_security_service(self): + share_server = db_utils.create_share_server() + security_service = db_utils.create_security_service() + + mock_ldap_bind = self.mock_object( + self.security_service_helper, 'ldap_bind') + + self.security_service_helper.setup_security_service( + share_server['id'], security_service) + + mock_ldap_bind.assert_called_once_with( + share_server['id'], security_service) + + def test_update_security_service(self): + share_server = db_utils.create_share_server() + current_security_service = db_utils.create_security_service() + new_security_service = db_utils.create_security_service() + + mock_ldap_bind = self.mock_object( + self.security_service_helper, 'ldap_bind') + + self.security_service_helper.update_security_service( + share_server['id'], current_security_service, new_security_service) + + mock_ldap_bind.assert_called_once_with( + share_server['id'], new_security_service) + + def _setup_test_ldap_bind_tests(self): + share_server = db_utils.create_security_service() + security_service = db_utils.create_security_service() + ldap_get_info = { + 'ss_password': security_service['password'], + 'ss_user': security_service['user'] + } + expected_cmd = [ + "docker", "exec", "%s" % share_server['id'], "ldapwhoami", "-x", + "-H", "ldap://localhost:389", "-D", + "cn=%s,dc=example,dc=com" % ldap_get_info[ + "ss_user"], + "-w", "%s" % ldap_get_info["ss_password"]] + + return share_server, security_service, ldap_get_info, expected_cmd + + def test_ldap_bind(self): + share_server, security_service, ldap_get_info, expected_cmd = ( + self._setup_test_ldap_bind_tests()) + + mock_ldap_get_info = self.mock_object( + self.security_service_helper, 'ldap_get_info', + mock.Mock(return_value=ldap_get_info)) + mock_ldap_retry_operation = self.mock_object( + self.security_service_helper, 'ldap_retry_operation') + + self.security_service_helper.ldap_bind( + share_server['id'], security_service) + + mock_ldap_get_info.assert_called_once_with(security_service) + mock_ldap_retry_operation.assert_called_once_with(expected_cmd, + run_as_root=True) + + def test_ldap_get_info(self): + security_service = db_utils.create_security_service() + expected_ldap_get_info = { + 'ss_password': security_service['password'], + 'ss_user': security_service['user'] + } + + ldap_get_info = self.security_service_helper.ldap_get_info( + security_service) + + self.assertEqual(expected_ldap_get_info, ldap_get_info) + + @ddt.data( + {'type': 'ldap'}, + {'user': 'fake_user'}, + {'password': 'fake_password'}, + ) + def test_ldap_get_info_exception(self, sec_service_data): + self.assertRaises( + exception.ShareBackendException, + self.security_service_helper.ldap_get_info, + sec_service_data + ) + + def test_ldap_retry_operation(self): + mock_cmd = ["command", "to", "be", "executed"] + + mock_execute = self.mock_object(self.security_service_helper, + '_execute') + + self.security_service_helper.ldap_retry_operation(mock_cmd, + run_as_root=True) + + mock_execute.assert_called_once_with(*mock_cmd, run_as_root=True) + + def test_ldap_retry_operation_timeout(self): + mock_cmd = ["command", "to", "be", "executed"] + + mock_execute = self.mock_object( + self.security_service_helper, '_execute', + mock.Mock( + side_effect=exception.ProcessExecutionError(exit_code=1))) + + self.assertRaises( + exception.ShareBackendException, + self.security_service_helper.ldap_retry_operation, + mock_cmd, + run_as_root=False, + timeout=10) + + mock_execute.assert_has_calls([ + mock.call(*mock_cmd, run_as_root=False), + mock.call(*mock_cmd, run_as_root=False)]) + + def test_ldap_retry_operation_invalid_credential(self): + mock_cmd = ["command", "to", "be", "executed"] + + mock_execute = self.mock_object( + self.security_service_helper, '_execute', + mock.Mock( + side_effect=exception.ProcessExecutionError( + exit_code=49))) + + self.assertRaises( + exception.ShareBackendException, + self.security_service_helper.ldap_retry_operation, + mock_cmd, + run_as_root=False) + + mock_execute.assert_called_once_with(*mock_cmd, run_as_root=False) diff --git a/releasenotes/notes/add-and-update-security-services-to-in-use-share-servers-on-container-driver-52193447c18e6d10.yaml b/releasenotes/notes/add-and-update-security-services-to-in-use-share-servers-on-container-driver-52193447c18e6d10.yaml new file mode 100644 index 0000000000..e0e4638c79 --- /dev/null +++ b/releasenotes/notes/add-and-update-security-services-to-in-use-share-servers-on-container-driver-52193447c18e6d10.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + The Container Driver is now able to handle LDAP security services + configuration while setting up share servers. Also, the Container Driver + allows adding or updating ``LDAP`` security services to in use share + networks.