Merge "[NetApp] Implement security service update"

manila/message/message_field.py

@@ -20,6 +20,7 @@ class Resource(object):
     SHARE_GROUP = 'SHARE_GROUP'
     SHARE_REPLICA = 'SHARE_REPLICA'
     SHARE_SNAPSHOT = 'SHARE_SNAPSHOT'
+    SECURITY_SERVICE = 'SECURITY_SERVICE'
 
 
 class Action(object):
@@ -34,6 +35,7 @@ class Action(object):
     EXTEND = ('008', _('extend'))
     SHRINK = ('009', _('shrink'))
     UPDATE_ACCESS_RULES = ('010', _('update access rules'))
+    ADD_UPDATE_SECURITY_SERVICE = ('011', _('add or update security service'))
     ALL = (
         ALLOCATE_HOST,
         CREATE,
@@ -45,6 +47,7 @@ class Action(object):
         EXTEND,
         SHRINK,
         UPDATE_ACCESS_RULES,
+        ADD_UPDATE_SECURITY_SERVICE,
     )
 
 
@@ -111,6 +114,12 @@ class Detail(object):
         _("Failed to grant access to client. The access level or type may "
           "be unsupported. You may try again with a different access level "
           "or access type."))
+    UNSUPPORTED_ADD_UDPATE_SECURITY_SERVICE = (
+        '022',
+        _("Share driver has failed to setup one or more security services "
+          "that are associated with the used share network. The security "
+          "service may be unsupported or the provided parameters are invalid. "
+          "You may try again with a different set of configurations."))
 
     ALL = (
         UNKNOWN_ERROR,
@@ -134,6 +143,7 @@ class Detail(object):
         DRIVER_FAILED_SHRINK,
         FORBIDDEN_CLIENT_ACCESS,
         UNSUPPORTED_CLIENT_ACCESS,
+        UNSUPPORTED_ADD_UDPATE_SECURITY_SERVICE,
     )
 
     # Exception and detail mappings

manila/share/drivers/netapp/dataontap/client/client_cmode.py

@@ -1535,7 +1535,6 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
             self.configure_dns(security_service)
 
         config_name = hashlib.md5(six.b(security_service['id'])).hexdigest()
-
         api_args = {
             'ldap-client-config': config_name,
             'tcp-port': '389',
@@ -1590,6 +1589,13 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
             LOG.exception(msg)
             raise exception.NetAppException(message=msg)
 
+    @na_utils.trace
+    def _delete_ldap_client(self, security_service):
+        config_name = (
+            hashlib.md5(six.b(security_service['id'])).hexdigest())
+        api_args = {'ldap-client-config': config_name}
+        self.send_request('ldap-client-delete', api_args)
+
     @na_utils.trace
     def configure_ldap(self, security_service, timeout=30):
         """Configures LDAP on Vserver."""
@@ -1598,17 +1604,59 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
         self._enable_ldap_client(config_name, timeout=timeout)
 
     @na_utils.trace
-    def configure_active_directory(self, security_service, vserver_name):
+    def modify_ldap(self, new_security_service, current_security_service):
-        """Configures AD on Vserver."""
+        """Modifies LDAP client on a Vserver."""
-        self.configure_dns(security_service)
+        # Create a new ldap client
-        self.set_preferred_dc(security_service)
+        self._create_ldap_client(new_security_service)
 
+        # Delete current ldap config
+        try:
+            self.send_request('ldap-config-delete')
+        except netapp_api.NaApiError as e:
+            if e.code != netapp_api.EOBJECTNOTFOUND:
+                msg = _("An error occurred while deleting original LDAP "
+                        "configuration. %s")
+                raise exception.NetAppException(msg % e.message)
+            else:
+                msg = _("Original LDAP configuration was not found. "
+                        "LDAP modification will continue.")
+                LOG.debug(msg)
+
+        new_config_name = (
+            hashlib.md5(six.b(new_security_service['id'])).hexdigest())
+        # Create ldap config with the new client
+        api_args = {'client-config': new_config_name, 'client-enabled': 'true'}
+        self.send_request('ldap-config-create', api_args)
+
+        # Delete old client configuration
+        try:
+            self._delete_ldap_client(current_security_service)
+        except netapp_api.NaApiError as e:
+            if e.code != netapp_api.EOBJECTNOTFOUND:
+                msg = _("An error occurred while deleting original LDAP "
+                        "client configuration. %s")
+                raise exception.NetAppException(msg % e.message)
+            else:
+                msg = _("Original LDAP client configuration was not found.")
+                LOG.debug(msg)
+
+    @na_utils.trace
+    def _get_cifs_server_name(self, vserver_name):
         # 'cifs-server' is CIFS Server NetBIOS Name, max length is 15.
         # Should be unique within each domain (data['domain']).
         # Cut to 15 char with begin and end, attempt to make valid DNS hostname
         cifs_server = (vserver_name[0:8] +
                        '-' +
                        vserver_name[-6:]).replace('_', '-').upper()
+        return cifs_server
+
+    @na_utils.trace
+    def configure_active_directory(self, security_service, vserver_name):
+        """Configures AD on Vserver."""
+        self.configure_dns(security_service)
+        self.set_preferred_dc(security_service)
+
+        cifs_server = self._get_cifs_server_name(vserver_name)
 
         api_args = {
             'admin-username': security_service['user'],
@@ -1628,6 +1676,46 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
             msg = _("Failed to create CIFS server entry. %s")
             raise exception.NetAppException(msg % e.message)
 
+    @na_utils.trace
+    def modify_active_directory_security_service(
+            self, vserver_name, differring_keys, new_security_service,
+            current_security_service):
+        cifs_server = self._get_cifs_server_name(vserver_name)
+
+        current_user_name = current_security_service['user']
+        new_username = new_security_service['user']
+
+        current_cifs_username = cifs_server + '\\' + current_user_name
+
+        if 'password' in differring_keys:
+            api_args = {
+                'user-name': current_cifs_username,
+                'user-password': new_security_service['password']
+            }
+            try:
+                self.send_request('cifs-local-user-set-password', api_args)
+            except netapp_api.NaApiError as e:
+                msg = _("Failed to modify existing CIFS server password. %s")
+                raise exception.NetAppException(msg % e.message)
+
+        if 'user' in differring_keys:
+            api_args = {
+                'user-name': current_cifs_username,
+                'new-user-name': new_username
+            }
+            try:
+                self.send_request('cifs-local-user-rename', api_args)
+            except netapp_api.NaApiError as e:
+                msg = _("Failed to modify existing CIFS server user-name. %s")
+                raise exception.NetAppException(msg % e.message)
+
+        if 'server' in differring_keys:
+            if current_security_service['server'] is not None:
+                self.remove_preferred_dcs(current_security_service)
+
+            if new_security_service['server'] is not None:
+                self.set_preferred_dc(new_security_service)
+
     @na_utils.trace
     def create_kerberos_realm(self, security_service):
         """Creates Kerberos realm on cluster."""
@@ -1694,6 +1782,26 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
                 security_service['domain'] + '@' +
                 security_service['domain'].upper())
 
+    @na_utils.trace
+    def update_kerberos_realm(self, security_service):
+        """Update Kerberos realm info. Only KDC IP can be changed."""
+        if not self.features.KERBEROS_VSERVER:
+            msg = _('Kerberos realms owned by Vserver are supported on ONTAP '
+                    '8.3 or later.')
+            raise exception.NetAppException(msg)
+
+        api_args = {
+            'admin-server-ip': security_service['server'],
+            'kdc-ip': security_service['server'],
+            'password-server-ip': security_service['server'],
+            'realm': security_service['domain'].upper(),
+        }
+        try:
+            self.send_request('kerberos-realm-modify', api_args)
+        except netapp_api.NaApiError as e:
+            msg = _('Failed to update Kerberos realm. %s')
+            raise exception.NetAppException(msg % e.message)
+
     @na_utils.trace
     def disable_kerberos(self, security_service):
         """Disable Kerberos in all Vserver LIFs."""
@@ -1819,6 +1927,36 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
                                  for server in servers.get_children()]
         return dns_config
 
+    @na_utils.trace
+    def update_dns_configuration(self, dns_ips, domains):
+        """Overrides DNS configuration with the specified IPs and domains."""
+        current_dns_config = self.get_dns_config()
+        api_args = {
+            'domains': [],
+            'name-servers': [],
+            'dns-state': 'enabled',
+        }
+        for domain in domains:
+            api_args['domains'].append({'string': domain})
+
+        for dns_ip in dns_ips:
+            api_args['name-servers'].append({'ip-address': dns_ip})
+
+        empty_dns_config = (not api_args['domains'] and
+                            not api_args['name-servers'])
+        if current_dns_config:
+            api_name, api_args = (
+                ('net-dns-destroy', {}) if empty_dns_config
+                else ('net-dns-modify', api_args))
+        else:
+            api_name, api_args = 'net-dns-create', api_args
+
+        try:
+            self.send_request(api_name, api_args)
+        except netapp_api.NaApiError as e:
+            msg = _("Failed to update DNS configuration. %s")
+            raise exception.NetAppException(msg % e.message)
+
     @na_utils.trace
     def set_preferred_dc(self, security_service):
         # server is optional
@@ -1842,6 +1980,20 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
             msg = _("Failed to set preferred DC. %s")
             raise exception.NetAppException(msg % e.message)
 
+    @na_utils.trace
+    def remove_preferred_dcs(self, security_service):
+        """Drops all preferred DCs at once."""
+
+        api_args = {
+            'domain': security_service['domain'],
+        }
+
+        try:
+            self.send_request('cifs-domain-preferred-dc-remove', api_args)
+        except netapp_api.NaApiError as e:
+            msg = _("Failed to unset preferred DCs. %s")
+            raise exception.NetAppException(msg % e.message)
+
     @na_utils.trace
     def create_volume(self, aggregate_name, volume_name, size_gb,
                       thin_provisioned=False, snapshot_policy=None,

manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py

@@ -35,6 +35,9 @@ class NetAppCmodeMultiSvmShareDriver(driver.ShareDriver):
             True, *args, **kwargs)
         self.library = lib_multi_svm.NetAppCmodeMultiSVMFileStorageLibrary(
             self.DRIVER_NAME, **kwargs)
+        # NetApp driver supports updating security service for in use share
+        # networks.
+        self.security_service_update_support = True
 
     def do_setup(self, context):
         self.library.do_setup(context)
@@ -336,3 +339,19 @@ class NetAppCmodeMultiSvmShareDriver(driver.ShareDriver):
                                             snapshots):
         return self.library.share_server_migration_get_progress(
             context, src_share_server, dest_share_server, shares, snapshots)
+
+    def update_share_server_security_service(
+            self, context, share_server, network_info, share_instances,
+            share_instance_rules, new_security_service,
+            current_security_service=None):
+        return self.library.update_share_server_security_service(
+            context, share_server, network_info, new_security_service,
+            current_security_service=current_security_service)
+
+    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):
+        return self.library.check_update_share_server_security_service(
+            context, share_server, network_info, new_security_service,
+            current_security_service=current_security_service)

manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py

@@ -319,3 +319,15 @@ class NetAppCmodeSingleSvmShareDriver(driver.ShareDriver):
             self, context, share_servers, share_group_ref,
             share_group_snapshot=None):
         raise NotImplementedError
+
+    def update_share_server_security_service(
+            self, context, share_server, network_info, share_instances,
+            share_instance_rules, new_security_service,
+            current_security_service=None):
+        raise NotImplementedError
+
+    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):
+        raise NotImplementedError

manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py

@@ -37,6 +37,7 @@ from manila.common import constants
 from manila import coordination
 from manila import exception
 from manila.i18n import _
+from manila.message import api as message_api
 from manila.share.drivers.netapp.dataontap.client import api as netapp_api
 from manila.share.drivers.netapp.dataontap.client import client_cmode
 from manila.share.drivers.netapp.dataontap.cluster_mode import data_motion
@@ -161,6 +162,7 @@ class NetAppCmodeFileStorageLibrary(object):
                                self.configuration.netapp_api_trace_pattern)
         self._backend_name = self.configuration.safe_get(
             'share_backend_name') or driver_name
+        self.message_api = message_api.API()
 
     @na_utils.trace
     def do_setup(self, context):
@@ -440,6 +442,7 @@ class NetAppCmodeFileStorageLibrary(object):
                 'snapshot_support': True,
                 'create_share_from_snapshot_support': True,
                 'revert_to_snapshot_support': self._revert_to_snapshot_support,
+                'security_service_update_support': True,
             }
 
             # Add storage service catalog data.

manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py

@@ -29,6 +29,7 @@ from oslo_utils import units
 
 from manila import exception
 from manila.i18n import _
+from manila.message import message_field
 from manila.share.drivers.netapp.dataontap.client import api as netapp_api
 from manila.share.drivers.netapp.dataontap.client import client_cmode
 from manila.share.drivers.netapp.dataontap.cluster_mode import data_motion
@@ -1365,3 +1366,157 @@ class NetAppCmodeMultiSVMFileStorageLibrary(
             .validate_provisioning_options_for_share(provisioning_options,
                                                      extra_specs=extra_specs,
                                                      qos_specs=qos_specs))
+
+    def _get_different_keys_for_equal_ss_type(self, current_sec_service,
+                                              new_sec_service):
+        different_keys = []
+
+        valid_keys = ['dns_ip', 'server', 'domain', 'user', 'password', 'ou']
+        for key, value in current_sec_service.items():
+            if (current_sec_service[key] != new_sec_service[key]
+                    and key in valid_keys):
+                different_keys.append(key)
+
+        return different_keys
+
+    def _is_security_service_valid(self, security_service):
+        mandatory_params = {
+            'ldap': ['user', 'password'],
+            'active_directory': ['dns_ip', 'domain', 'user', 'password'],
+            'kerberos': ['dns_ip', 'domain', 'user', 'password', 'server'],
+        }
+        ss_type = security_service['type']
+        if ss_type == 'ldap':
+            ad_domain = security_service.get('domain')
+            ldap_servers = security_service.get('server')
+            if not bool(ad_domain) ^ bool(ldap_servers):
+                msg = _("LDAP security service must have either 'server' or "
+                        "'domain' parameters. Use 'server' for Linux/Unix "
+                        "LDAP servers or 'domain' for Active Directory LDAP "
+                        "server.")
+                LOG.error(msg)
+                return False
+
+        if not all([security_service[key] is not None
+                    for key in mandatory_params[ss_type]]):
+            msg = _("The security service %s does not have all the "
+                    "parameters needed to used by the share driver."
+                    ) % security_service['id']
+            LOG.error(msg)
+            return False
+
+        return True
+
+    def update_share_server_security_service(self, context, share_server,
+                                             network_info,
+                                             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()
+
+        vserver_name, vserver_client = self._get_vserver(
+            share_server=share_server)
+
+        # Check if this update is supported by our driver
+        if not self.check_update_share_server_security_service(
+                context, share_server, network_info, new_security_service,
+                current_security_service=current_security_service):
+            msg = _("The requested security service update is not supported "
+                    "by the NetApp driver.")
+            LOG.exception(msg)
+            raise exception.NetAppException(msg)
+
+        if current_security_service is None:
+            self._client.setup_security_services([new_security_service],
+                                                 vserver_client,
+                                                 vserver_name)
+            LOG.info("A new security service configuration was added to share "
+                     "server '%(share_server_id)s'",
+                     {'share_server_id': share_server['id']})
+            return
+
+        different_keys = self._get_different_keys_for_equal_ss_type(
+            current_security_service, new_security_service)
+        if not different_keys:
+            msg = _("The former and the latter security services are "
+                    "equal. Nothing to do.")
+            LOG.debug(msg)
+            return
+
+        if 'dns_ip' in different_keys:
+            dns_ips = set()
+            domains = set()
+            # Read all dns-ips and domains from other security services
+            for sec_svc in network_info['security_services']:
+                if sec_svc['type'] == current_type:
+                    # skip the one that we are replacing
+                    continue
+                if sec_svc.get('dns_ip') is not None:
+                    for dns_ip in sec_svc['dns_ip'].split(','):
+                        dns_ips.add(dns_ip.strip())
+                if sec_svc.get('domain') is not None:
+                    domains.add(sec_svc['domain'])
+            # Merge with the new dns configuration
+            if new_security_service.get('dns_ip') is not None:
+                for dns_ip in new_security_service['dns_ip'].split(','):
+                    dns_ips.add(dns_ip.strip())
+            if new_security_service.get('domain') is not None:
+                domains.add(new_security_service['domain'])
+
+            # Update vserver DNS configuration
+            vserver_client.update_dns_configuration(dns_ips, domains)
+
+        if new_type == 'kerberos':
+            if 'server' in different_keys:
+                # NOTE(dviroel): Only IPs will be updated here, new principals
+                # won't be configured here. It is expected that only the IP was
+                # changed, but the KDC remains the same.
+                LOG.debug('Updating kerberos realm on NetApp backend.')
+                vserver_client.update_kerberos_realm(new_security_service)
+
+        elif new_type == 'active_directory':
+            vserver_client.modify_active_directory_security_service(
+                vserver_name, different_keys, new_security_service,
+                current_security_service)
+        else:
+            vserver_client.modify_ldap(new_security_service,
+                                       current_security_service)
+
+        LOG.info("Security service configuration was updated for share server "
+                 "'%(share_server_id)s'",
+                 {'share_server_id': share_server['id']})
+
+    def check_update_share_server_security_service(
+            self, context, share_server, network_info,
+            new_security_service, current_security_service=None):
+        current_type = (
+            current_security_service['type'].lower()
+            if current_security_service else '')
+
+        if not self._is_security_service_valid(new_security_service):
+            self.message_api.create(
+                context,
+                message_field.Action.ADD_UPDATE_SECURITY_SERVICE,
+                new_security_service['project_id'],
+                resource_type=message_field.Resource.SECURITY_SERVICE,
+                resource_id=new_security_service['id'],
+                detail=(message_field.Detail
+                        .UNSUPPORTED_ADD_UDPATE_SECURITY_SERVICE))
+            return False
+
+        if current_security_service:
+            if current_type != 'ldap':
+                # NOTE(dviroel): We don't support domain/realm updates for
+                # Kerberos security service, because it might require a new SPN
+                # to be created and to destroy the old one, thus disrupting all
+                # shares hosted by this share server. Same issue can happen
+                # with AD domain modifications.
+                if (current_security_service['domain'].lower() !=
+                        new_security_service['domain'].lower()):
+                    msg = _("Currently the driver does not support updates "
+                            "in the security service 'domain'.")
+                    LOG.info(msg)
+                    return False
+        return True

manila/tests/share/drivers/netapp/dataontap/client/fakes.py

@@ -471,7 +471,17 @@ CIFS_SECURITY_SERVICE = {
     'ou': 'fake_ou',
     'domain': 'fake_domain',
     'dns_ip': 'fake_dns_ip',
-    'server': '',
+    'server': 'fake_server',
+}
+
+CIFS_SECURITY_SERVICE_2 = {
+    'type': 'active_directory',
+    'password': 'fake_password_2',
+    'user': 'fake_user_2',
+    'ou': 'fake_ou_2',
+    'domain': 'fake_domain_2',
+    'dns_ip': 'fake_dns_ip_2',
+    'server': 'fake_server_2',
 }
 
 LDAP_LINUX_SECURITY_SERVICE = {
@@ -496,6 +506,7 @@ LDAP_AD_SECURITY_SERVICE = {
     'server': None,
 }
 
+
 KERBEROS_SECURITY_SERVICE = {
     'type': 'kerberos',
     'password': 'fake_password',

manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py

@@ -2670,6 +2670,42 @@ class NetAppClientCmodeTestCase(test.TestCase):
                           self.client.create_kerberos_realm,
                           fake.KERBEROS_SECURITY_SERVICE)
 
+    def test_update_kerberos_realm(self):
+        self.client.features.add_feature('KERBEROS_VSERVER')
+        self.mock_object(self.client, 'send_request')
+
+        self.client.update_kerberos_realm(fake.KERBEROS_SECURITY_SERVICE)
+
+        kerberos_realm_create_args = {
+            'admin-server-ip': fake.KERBEROS_SECURITY_SERVICE['server'],
+            'kdc-ip': fake.KERBEROS_SECURITY_SERVICE['server'],
+            'password-server-ip': fake.KERBEROS_SECURITY_SERVICE['server'],
+            'realm': fake.KERBEROS_SECURITY_SERVICE['domain'].upper(),
+        }
+
+        self.client.send_request.assert_has_calls([
+            mock.call('kerberos-realm-modify',
+                      kerberos_realm_create_args)])
+
+    def test_update_kerberos_realm_failure(self):
+        self.client.features.add_feature('KERBEROS_VSERVER')
+        self.mock_object(self.client, 'send_request', self._mock_api_error())
+
+        self.assertRaises(exception.NetAppException,
+                          self.client.update_kerberos_realm,
+                          fake.KERBEROS_SECURITY_SERVICE)
+
+        kerberos_realm_create_args = {
+            'admin-server-ip': fake.KERBEROS_SECURITY_SERVICE['server'],
+            'kdc-ip': fake.KERBEROS_SECURITY_SERVICE['server'],
+            'password-server-ip': fake.KERBEROS_SECURITY_SERVICE['server'],
+            'realm': fake.KERBEROS_SECURITY_SERVICE['domain'].upper(),
+        }
+
+        self.client.send_request.assert_has_calls([
+            mock.call('kerberos-realm-modify',
+                      kerberos_realm_create_args)])
+
     def test_configure_kerberos(self):
         self.client.features.add_feature('KERBEROS_VSERVER')
         self.mock_object(self.client, 'send_request')
@@ -2880,6 +2916,34 @@ class NetAppClientCmodeTestCase(test.TestCase):
         self.client.send_request.assert_has_calls([
             mock.call('net-dns-modify', net_dns_create_args)])
 
+    def test_update_dns_configuration(self):
+        fake_configured_dns = {
+            'dns-state': 'enabled',
+            'domains': ['fake_domain_2'],
+            'dns-ips': ['fake_dns_ip_2']
+        }
+        self.mock_object(self.client, 'get_dns_config',
+                         mock.Mock(return_value=fake_configured_dns))
+        self.mock_object(self.client, 'send_request')
+
+        self.client.configure_dns(fake.KERBEROS_SECURITY_SERVICE)
+        domains = set()
+        domains.add(fake_configured_dns['domains'][0])
+        domains.add(fake.KERBEROS_SECURITY_SERVICE['domain'])
+
+        dns_ips = set()
+        dns_ips.add(fake_configured_dns['dns-ips'][0])
+        dns_ips.add(fake.KERBEROS_SECURITY_SERVICE['dns_ip'])
+
+        net_dns_create_args = {
+            'domains': [{'string': domain} for domain in domains],
+            'dns-state': 'enabled',
+            'name-servers': [{'ip-address': dns_ip} for dns_ip in dns_ips]
+        }
+
+        self.client.send_request.assert_has_calls([
+            mock.call('net-dns-modify', net_dns_create_args)])
+
     def test_configure_dns_api_error(self):
 
         self.mock_object(self.client, 'send_request', self._mock_api_error())
@@ -2960,6 +3024,34 @@ class NetAppClientCmodeTestCase(test.TestCase):
                           self.client.set_preferred_dc,
                           security_service)
 
+    def test_remove_preferred_dcs(self):
+        self.mock_object(self.client, 'send_request')
+        security_service = copy.deepcopy(fake.CIFS_SECURITY_SERVICE)
+
+        self.client.remove_preferred_dcs(security_service)
+
+        preferred_dc_add_args = {
+            'domain': security_service['domain'],
+        }
+        self.client.send_request.assert_has_calls([
+            mock.call('cifs-domain-preferred-dc-remove',
+                      preferred_dc_add_args)])
+
+    def test_remove_preferred_dcs_error(self):
+        self.mock_object(self.client, 'send_request', self._mock_api_error())
+        security_service = copy.deepcopy(fake.CIFS_SECURITY_SERVICE)
+
+        self.assertRaises(exception.NetAppException,
+                          self.client.remove_preferred_dcs,
+                          security_service)
+
+        preferred_dc_add_args = {
+            'domain': security_service['domain'],
+        }
+        self.client.send_request.assert_has_calls([
+            mock.call('cifs-domain-preferred-dc-remove',
+                      preferred_dc_add_args)])
+
     def test_create_volume(self):
 
         self.mock_object(self.client, 'send_request')
@@ -7622,6 +7714,114 @@ class NetAppClientCmodeTestCase(test.TestCase):
         self.client.send_request.assert_called_once_with(
             'volume-autosize-get', {'volume': fake.SHARE_NAME})
 
+    def test_modify_active_directory_security_service(self):
+        curr_sec_service = copy.deepcopy(fake.CIFS_SECURITY_SERVICE)
+        new_sec_service = copy.deepcopy(fake.CIFS_SECURITY_SERVICE_2)
+        # we don't support domain change, but this validation isn't made in
+        # within this method
+        new_sec_service['domain'] = curr_sec_service['domain']
+        api_responses = [fake.PASSED_RESPONSE, fake.PASSED_RESPONSE]
+
+        self.mock_object(self.client, 'send_request',
+                         mock.Mock(side_effect=api_responses))
+        self.mock_object(self.client, 'remove_preferred_dcs')
+        self.mock_object(self.client, 'set_preferred_dc')
+        differing_keys = {'password', 'user', 'server'}
+
+        self.client.modify_active_directory_security_service(
+            fake.VSERVER_NAME, differing_keys, new_sec_service,
+            curr_sec_service)
+
+        cifs_server = self.client._get_cifs_server_name(fake.VSERVER_NAME)
+        current_cifs_username = cifs_server + '\\' + curr_sec_service['user']
+        set_pass_api_args = {
+            'user-name': current_cifs_username,
+            'user-password': new_sec_service['password']
+        }
+        user_rename_api_args = {
+            'user-name': current_cifs_username,
+            'new-user-name': new_sec_service['user']
+        }
+
+        self.client.send_request.assert_has_calls([
+            mock.call('cifs-local-user-set-password', set_pass_api_args),
+            mock.call('cifs-local-user-rename', user_rename_api_args)])
+        self.client.remove_preferred_dcs.assert_called_once_with(
+            curr_sec_service)
+        self.client.set_preferred_dc.assert_called_once_with(new_sec_service)
+
+    @ddt.data(True, False)
+    def test_modify_active_directory_security_service_error(
+            self, cifs_set_password_failure):
+        curr_sec_service = copy.deepcopy(fake.CIFS_SECURITY_SERVICE)
+        new_sec_service = copy.deepcopy(fake.CIFS_SECURITY_SERVICE_2)
+        # we don't support domain change, but this validation isn't made in
+        # within this method
+        new_sec_service['domain'] = curr_sec_service['domain']
+        if cifs_set_password_failure:
+            api_responses = [netapp_api.NaApiError(code='fake'),
+                             fake.PASSED_RESPONSE]
+        else:
+            api_responses = [fake.PASSED_RESPONSE,
+                             netapp_api.NaApiError(code='fake')]
+
+        self.mock_object(self.client, 'send_request',
+                         mock.Mock(side_effect=api_responses))
+        differing_keys = {'password', 'user', 'server'}
+
+        self.assertRaises(
+            exception.NetAppException,
+            self.client.modify_active_directory_security_service,
+            fake.VSERVER_NAME, differing_keys, new_sec_service,
+            curr_sec_service)
+
+        cifs_server = self.client._get_cifs_server_name(fake.VSERVER_NAME)
+        current_cifs_username = cifs_server + '\\' + curr_sec_service['user']
+        set_pass_api_args = {
+            'user-name': current_cifs_username,
+            'user-password': new_sec_service['password']
+        }
+        user_rename_api_args = {
+            'user-name': current_cifs_username,
+            'new-user-name': new_sec_service['user']
+        }
+
+        if cifs_set_password_failure:
+            send_request_calls = [
+                mock.call('cifs-local-user-set-password', set_pass_api_args)]
+        else:
+            send_request_calls = [
+                mock.call('cifs-local-user-set-password', set_pass_api_args),
+                mock.call('cifs-local-user-rename', user_rename_api_args)
+            ]
+
+        self.client.send_request.assert_has_calls(send_request_calls)
+
+    @ddt.data(False, True)
+    def test_modify_ldap(self, api_not_found):
+        current_ldap_service = fake.LDAP_AD_SECURITY_SERVICE
+        new_ldap_service = fake.LDAP_LINUX_SECURITY_SERVICE
+        config_name = hashlib.md5(six.b(new_ldap_service['id'])).hexdigest()
+        api_result = (self._mock_api_error(code=netapp_api.EOBJECTNOTFOUND)
+                      if api_not_found else mock.Mock())
+        mock_create_client = self.mock_object(
+            self.client, '_create_ldap_client')
+        mock_send_request = self.mock_object(
+            self.client, 'send_request',
+            mock.Mock(return_value=api_result))
+        mock_delete_client = self.mock_object(
+            self.client, '_delete_ldap_client',
+            mock.Mock(return_value=api_result))
+
+        self.client.modify_ldap(new_ldap_service, current_ldap_service)
+
+        api_args = {'client-config': config_name, 'client-enabled': 'true'}
+        mock_create_client.assert_called_once_with(new_ldap_service)
+        mock_send_request.assert_has_calls([
+            mock.call('ldap-config-delete'),
+            mock.call('ldap-config-create', api_args)])
+        mock_delete_client.assert_called_once_with(current_ldap_service)
+
     def test_create_fpolicy_event(self):
         self.mock_object(self.client, 'send_request')
 

manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py

@@ -2627,3 +2627,164 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
             exception.NetAppException,
             self.library.validate_provisioning_options_for_share,
             fake.PROVISIONING_OPTS_WITH_ADAPT_QOS, qos_specs=None)
+
+    def test__get_different_keys_for_equal_ss_type(self):
+        curr_sec_service = copy.deepcopy(fake.CIFS_SECURITY_SERVICE)
+        new_sec_service = copy.deepcopy(fake.CIFS_SECURITY_SERVICE_2)
+
+        expected_keys = ['password', 'user', 'ou',
+                         'domain', 'dns_ip', 'server']
+
+        result = self.library._get_different_keys_for_equal_ss_type(
+            curr_sec_service, new_sec_service)
+
+        self.assertEqual(expected_keys, result)
+
+    @ddt.data(
+        {'current': None,
+         'new': fake.CIFS_SECURITY_SERVICE,
+         'existing': []},
+
+        {'current': fake.CIFS_SECURITY_SERVICE,
+         'new': fake.CIFS_SECURITY_SERVICE_2,
+         'existing': [fake.CIFS_SECURITY_SERVICE,
+                      fake.KERBEROS_SECURITY_SERVICE]},
+
+        {'current': fake.KERBEROS_SECURITY_SERVICE,
+         'new': fake.KERBEROS_SECURITY_SERVICE_2,
+         'existing': [fake.CIFS_SECURITY_SERVICE,
+                      fake.KERBEROS_SECURITY_SERVICE]},
+
+        {'current': fake.CIFS_SECURITY_SERVICE,
+         'new': fake.CIFS_SECURITY_SERVICE,
+         'existing': [fake.CIFS_SECURITY_SERVICE]},
+    )
+    @ddt.unpack
+    def test_update_share_server_security_service(self, current, new,
+                                                  existing):
+        fake_context = mock.Mock()
+        fake_net_info = copy.deepcopy(fake.NETWORK_INFO)
+        new_sec_service = copy.deepcopy(new)
+        curr_sec_service = copy.deepcopy(current) if current else None
+        new_type = new_sec_service['type'].lower()
+        fake_net_info['security_services'] = existing
+
+        if curr_sec_service:
+            # domain modification aren't support
+            new_sec_service['domain'] = curr_sec_service['domain']
+
+        different_keys = []
+        if curr_sec_service != new_sec_service:
+            different_keys = ['dns_ip', 'server', 'domain', 'user', 'password']
+            if new_sec_service.get('ou') is not None:
+                different_keys.append('ou')
+
+        fake_vserver_client = mock.Mock()
+        mock_get_vserver = self.mock_object(
+            self.library, '_get_vserver',
+            mock.Mock(return_value=[fake.VSERVER1, fake_vserver_client]))
+        mock_check_update = self.mock_object(
+            self.library, 'check_update_share_server_security_service',
+            mock.Mock(return_value=True))
+        mock_setup_sec_serv = self.mock_object(
+            self.library._client, 'setup_security_services')
+        mock_diff_keys = self.mock_object(
+            self.library, '_get_different_keys_for_equal_ss_type',
+            mock.Mock(return_value=different_keys))
+        mock_dns_update = self.mock_object(
+            fake_vserver_client, 'update_dns_configuration')
+        mock_update_krealm = self.mock_object(
+            fake_vserver_client, 'update_kerberos_realm')
+        mock_modify_ad = self.mock_object(
+            fake_vserver_client, 'modify_active_directory_security_service')
+
+        self.library.update_share_server_security_service(
+            fake_context, fake.SHARE_SERVER, fake_net_info,
+            new_sec_service, current_security_service=curr_sec_service)
+
+        dns_ips = set()
+        domains = set()
+        # we don't need to split and strip since we know that fake have only
+        # on dns-ip and domain configured
+        for ss in existing:
+            if ss['type'] != new_sec_service['type']:
+                dns_ips.add(ss['dns_ip'])
+                domains.add(ss['domain'])
+        dns_ips.add(new_sec_service['dns_ip'])
+        domains.add(new_sec_service['domain'])
+
+        mock_get_vserver.assert_called_once_with(
+            share_server=fake.SHARE_SERVER)
+        mock_check_update.assert_called_once_with(
+            fake_context, fake.SHARE_SERVER, fake_net_info, new_sec_service,
+            current_security_service=curr_sec_service)
+
+        if curr_sec_service is None:
+            mock_setup_sec_serv.assert_called_once_with(
+                [new_sec_service], fake_vserver_client, fake.VSERVER1)
+        else:
+            mock_diff_keys.assert_called_once_with(curr_sec_service,
+                                                   new_sec_service)
+            if different_keys:
+                mock_dns_update.assert_called_once_with(dns_ips, domains)
+                if new_type == 'kerberos':
+                    mock_update_krealm.assert_called_once_with(new_sec_service)
+                elif new_type == 'active_directory':
+                    mock_modify_ad.assert_called_once_with(
+                        fake.VSERVER1, different_keys, new_sec_service,
+                        curr_sec_service)
+
+    def test_update_share_server_security_service_check_error(self):
+        curr_sec_service = copy.deepcopy(fake.CIFS_SECURITY_SERVICE)
+        new_sec_service = copy.deepcopy(fake.CIFS_SECURITY_SERVICE_2)
+        fake_vserver_client = mock.Mock()
+        fake_context = mock.Mock()
+        fake_net_info = mock.Mock()
+
+        mock_get_vserver = self.mock_object(
+            self.library, '_get_vserver',
+            mock.Mock(return_value=[fake.VSERVER1, fake_vserver_client]))
+        mock_check_update = self.mock_object(
+            self.library, 'check_update_share_server_security_service',
+            mock.Mock(return_value=False))
+
+        self.assertRaises(
+            exception.NetAppException,
+            self.library.update_share_server_security_service,
+            fake_context, fake.SHARE_SERVER, fake_net_info,
+            new_sec_service, current_security_service=curr_sec_service)
+
+        mock_get_vserver.assert_called_once_with(
+            share_server=fake.SHARE_SERVER)
+        mock_check_update.assert_called_once_with(
+            fake_context, fake.SHARE_SERVER, fake_net_info,
+            new_sec_service, current_security_service=curr_sec_service)
+
+    @ddt.data(
+        {'new': fake.LDAP_AD_SECURITY_SERVICE,
+         'current': fake.LDAP_LINUX_SECURITY_SERVICE,
+         'expected': True},
+
+        {'new': fake.CIFS_SECURITY_SERVICE,
+         'current': fake.KERBEROS_SECURITY_SERVICE,
+         'expected': False},
+
+        {'new': fake.CIFS_SECURITY_SERVICE,
+         'current': fake.CIFS_SECURITY_SERVICE,
+         'expected': True},
+
+        {'new': fake.KERBEROS_SECURITY_SERVICE,
+         'current': fake.KERBEROS_SECURITY_SERVICE,
+         'expected': True},
+
+        {'new': fake.CIFS_SECURITY_SERVICE,
+         'current': None,
+         'expected': True},
+    )
+    @ddt.unpack
+    def test_check_update_share_server_security_service(self, new, current,
+                                                        expected):
+        result = self.library.check_update_share_server_security_service(
+            None, None, None, new, current_security_service=current)
+
+        self.assertEqual(expected, result)

manila/tests/share/drivers/netapp/dataontap/fakes.py

@@ -911,6 +911,7 @@ POOLS = [
         'create_share_from_snapshot_support': True,
         'revert_to_snapshot_support': True,
         'qos': True,
+        'security_service_update_support': True,
     },
     {
         'pool_name': AGGREGATES[1],
@@ -934,6 +935,7 @@ POOLS = [
         'create_share_from_snapshot_support': True,
         'revert_to_snapshot_support': True,
         'qos': True,
+        'security_service_update_support': True,
     },
 ]
 
@@ -957,6 +959,7 @@ POOLS_VSERVER_CREDS = [
         'create_share_from_snapshot_support': True,
         'revert_to_snapshot_support': True,
         'qos': False,
+        'security_service_update_support': True,
     },
     {
         'pool_name': AGGREGATES[1],
@@ -977,6 +980,7 @@ POOLS_VSERVER_CREDS = [
         'create_share_from_snapshot_support': True,
         'revert_to_snapshot_support': True,
         'qos': False,
+        'security_service_update_support': True,
     },
 ]
 
@@ -1555,7 +1559,60 @@ CIFS_SECURITY_SERVICE = {
     'ou': 'fake_ou',
     'domain': 'fake_domain',
     'dns_ip': 'fake_dns_ip',
-    'server': '',
+    'server': 'fake_server',
+}
+
+CIFS_SECURITY_SERVICE_2 = {
+    'id': 'fake_id_2',
+    'type': 'active_directory',
+    'password': 'fake_password_2',
+    'user': 'fake_user_2',
+    'ou': 'fake_ou_2',
+    'domain': 'fake_domain_2',
+    'dns_ip': 'fake_dns_ip_2',
+    'server': 'fake_server_2',
+}
+LDAP_LINUX_SECURITY_SERVICE = {
+    'id': 'fake_id',
+    'type': 'ldap',
+    'user': 'fake_user',
+    'password': 'fake_password',
+    'server': 'fake_server',
+    'ou': 'fake_ou',
+    'dns_ip': None,
+    'domain': None
+}
+
+
+LDAP_AD_SECURITY_SERVICE = {
+    'id': 'fake_id',
+    'type': 'ldap',
+    'user': 'fake_user',
+    'password': 'fake_password',
+    'domain': 'fake_domain',
+    'ou': 'fake_ou',
+    'dns_ip': 'fake_dns_ip',
+    'server': None,
+}
+
+KERBEROS_SECURITY_SERVICE = {
+    'id': 'fake_id_3',
+    'type': 'kerberos',
+    'password': 'fake_password_3',
+    'user': 'fake_user_3',
+    'domain': 'fake_realm',
+    'dns_ip': 'fake_dns_ip_3',
+    'server': 'fake_server_3',
+}
+
+KERBEROS_SECURITY_SERVICE_2 = {
+    'id': 'fake_id_4',
+    'type': 'kerberos',
+    'password': 'fake_password_4',
+    'user': 'fake_user_4',
+    'domain': 'fake_realm_2',
+    'dns_ip': 'fake_dns_ip_4',
+    'server': 'fake_server_4',
 }
 
 SHARE_NETWORK_SUBNET = {

releasenotes/notes/netapp-add-security-service-update-718a68ebe60fd2b5.yaml

@@ -0,0 +1,10 @@
+---
+features:
+  - |
+    NetApp ONTAP driver now supports add and update security services when
+    they are associated with in use share networks. Both add and update
+    operations are supported by all three security service types:
+    ``active_directory``, ``kerberos`` and ``ldap``. In order to update their
+    parameters in a non-disruptively way, ``active_directory`` and ``kerberos``
+    don't support ``domain`` updates.
+