diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index b5aeb868bf..46dfefb36d 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -213,13 +213,15 @@ * 2.93 - Added support for filtering services by 'ensuring'. * 2.94 - Added QoS type and specs APIs. * 2.95 - Added Share Replica Metadata to Metadata API + * 2.96 - Added dns_name and dns_domain fields to export location API + responses when the share has DNS metadata set. """ # The minimum and maximum versions of the API supported # The default api version request is defined to be the # minimum version of the API supported. _MIN_API_VERSION = "2.0" -_MAX_API_VERSION = "2.95" +_MAX_API_VERSION = "2.96" DEFAULT_API_VERSION = _MIN_API_VERSION diff --git a/manila/api/v2/shares.py b/manila/api/v2/shares.py index 7ae03cbc57..5cf1e20a19 100644 --- a/manila/api/v2/shares.py +++ b/manila/api/v2/shares.py @@ -37,6 +37,7 @@ from manila.lock import api as resource_locks from manila import policy from manila import share +from manila.share import api as share_api from manila import utils LOG = log.getLogger(__name__) @@ -676,6 +677,11 @@ def _validate_metadata_for_update(self, req, share_id, metadata, _metadata = current_share_metadata.copy() _metadata.update(metadata_copy) + try: + share_api._validate_dns_metadata(_metadata) + except exception.InvalidInput as e: + raise exc.HTTPBadRequest(explanation=e.msg) + return _metadata # NOTE: (ashrod98) original metadata method and policy overrides @@ -747,7 +753,21 @@ def delete_metadata(self, req, resource_id, key): if key in self._conf_admin_only_metadata_keys: policy.check_policy(context, 'share', 'update_admin_only_metadata') - return self._delete_metadata(req, resource_id, key) + + if key in ('dns_name', 'dns_domain'): + current = self.share_api.db.share_metadata_get( + context, resource_id) + other_key = 'dns_domain' if key == 'dns_name' else 'dns_name' + if current.get(other_key): + raise exc.HTTPBadRequest(explanation=_( + "'dns_name' and 'dns_domain' must be deleted together. " + "Delete both keys or use metadata update to change them.")) + + pre_delete_metadata = self.share_api.db.share_metadata_get( + context, resource_id) + self._delete_metadata(req, resource_id, key) + self.share_api.update_share_from_metadata( + context, resource_id, pre_delete_metadata) def create_resource(): diff --git a/manila/api/views/export_locations.py b/manila/api/views/export_locations.py index 8f2e787b9a..c40ae375bf 100644 --- a/manila/api/views/export_locations.py +++ b/manila/api/views/export_locations.py @@ -28,6 +28,7 @@ class ViewBuilder(common.ViewBuilder): _detail_version_modifiers = [ 'add_preferred_path_attribute', 'add_metadata_attribute', + 'add_dns_metadata_attributes', ] def _get_export_location_view(self, request, export_location, @@ -97,4 +98,17 @@ def add_metadata_attribute(self, context, view_dict, metadata = export_location.get('el_metadata') meta_copy = copy.copy(metadata) meta_copy.pop('preferred', None) + meta_copy.pop('dns_name', None) + meta_copy.pop('dns_domain', None) view_dict['metadata'] = meta_copy + + @common.ViewBuilder.versioned_method('2.96') + def add_dns_metadata_attributes(self, context, view_dict, export_location): + el_metadata = export_location.get('el_metadata', {}) + dns_name = el_metadata.get('dns_name') + dns_domain = el_metadata.get('dns_domain') + + if dns_name: + view_dict['dns_name'] = dns_name + if dns_domain: + view_dict['dns_domain'] = dns_domain diff --git a/manila/dns/designate.py b/manila/dns/designate.py new file mode 100644 index 0000000000..2146b84e9f --- /dev/null +++ b/manila/dns/designate.py @@ -0,0 +1,224 @@ +# Copyright 2026 SAP SE. +# 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 keystoneauth1 import loading as ks_loading +from oslo_config import cfg +from oslo_log import log +from oslo_utils import importutils +from oslo_utils import timeutils + +from manila.common import client_auth +from manila import exception + +_designateclient_module = importutils.try_import('designateclient.v2.client') + +LOG = log.getLogger(__name__) + +DESIGNATE_GROUP = 'designate' +AUTH_OBJ = None + +_ZONE_CACHE_TTL_SECONDS = 300 + +designate_opts = [ + cfg.BoolOpt( + 'enabled', + default=False, + help='Enable Designate DNS integration for shares with DNS metadata.'), + cfg.StrOpt( + 'endpoint_type', + default='publicURL', + help='Endpoint type to be used with Designate client calls.'), + cfg.StrOpt( + 'region_name', + help='Region name for connecting to Designate.'), + cfg.IntOpt( + 'ttl', + default=300, + help='TTL in seconds for DNS A records created by Manila.'), +] + +CONF = cfg.CONF +CONF.register_opts(designate_opts, DESIGNATE_GROUP) +ks_loading.register_session_conf_options(CONF, DESIGNATE_GROUP) +ks_loading.register_auth_conf_options(CONF, DESIGNATE_GROUP) + + +def list_opts(): + return client_auth.AuthClientLoader.list_opts(DESIGNATE_GROUP) + + +def designateclient(context): + """Get authenticated Designate client using service credentials.""" + if _designateclient_module is None: + raise ImportError( + "python-designateclient is required for Designate integration. " + "Install it with: pip install python-designateclient") + global AUTH_OBJ + if not AUTH_OBJ: + AUTH_OBJ = client_auth.AuthClientLoader( + client_class=_designateclient_module.Client, + cfg_group=DESIGNATE_GROUP) + return AUTH_OBJ.get_client( + context, + admin=True, + endpoint_type=CONF[DESIGNATE_GROUP].endpoint_type, + region_name=CONF[DESIGNATE_GROUP].region_name, + ) + + +class API(object): + """API for interacting with Designate for DNS record management.""" + + def __init__(self): + self._enabled = CONF[DESIGNATE_GROUP].enabled + self._client = None + self._zone_id_cache = {} + + @property + def enabled(self): + """Returns True if Designate integration is enabled.""" + return self._enabled + + def _get_client(self, context): + """Get and cache the Designate client.""" + + if self._client is None: + self._client = designateclient(context) + return self._client + + def _get_zone_id(self, context, client, dns_domain, force_refresh=False): + """Find Designate zone ID by domain name, scoped to project. + + Looks up the zone dynamically so that different shares can use + different DNS domains without a hardcoded zone_id in the config. + The lookup is filtered by the caller's project_id to prevent a + privileged Manila service from writing into another project's zone. + + Results are cached for _ZONE_CACHE_TTL_SECONDS seconds. Pass + force_refresh=True to bypass the cache (e.g. after a NotFound on + write, which indicates the zone was recreated with a new ID). + """ + project_id = context.project_id + cache_key = (project_id, dns_domain) + if not force_refresh and cache_key in self._zone_id_cache: + zone_id, cached_at = self._zone_id_cache[cache_key] + age = timeutils.utcnow_ts() - cached_at + if age < _ZONE_CACHE_TTL_SECONDS: + return zone_id + # Cache expired — fall through to re-lookup below. + del self._zone_id_cache[cache_key] + + name = dns_domain if dns_domain.endswith('.') else dns_domain + '.' + criterion = {'name': name} + if project_id: + criterion['project_id'] = project_id + zones = client.zones.list(criterion=criterion) + if not zones: + raise exception.NotFound( + "Designate zone for domain '%s' not found in project '%s'. " + "Ensure the zone exists and the Manila service account " + "has access to it." % (dns_domain, project_id)) + zone_id = zones[0]['id'] + self._zone_id_cache[cache_key] = (zone_id, timeutils.utcnow_ts()) + return zone_id + + def create_record(self, context, dns_name, dns_domain, ip_addresses): + """Create a DNS A recordset in Designate.""" + + if not self.enabled: + return None + + fqdn = '%s.%s' % (dns_name, dns_domain) + client = self._get_client(context) + zone_id = self._get_zone_id(context, client, dns_domain) + try: + recordset = client.recordsets.create( + zone_id, + fqdn, + 'A', + ip_addresses, + ttl=CONF[DESIGNATE_GROUP].ttl, + ) + except Exception: + # Invalidate the cached zone_id in case it became stale. + self._zone_id_cache.pop((context.project_id, dns_domain), None) + LOG.exception("Failed to create DNS A record '%s' -> %s.", + fqdn, ip_addresses) + raise + LOG.info("Created DNS A record '%s' -> %s (recordset %s)", + fqdn, ip_addresses, recordset['id']) + return recordset['id'] + + def delete_record(self, context, dns_name, dns_domain): + """Delete a DNS A recordset from Designate.""" + + if not self.enabled: + return True + + fqdn = '%s.%s' % (dns_name, dns_domain) + try: + client = self._get_client(context) + zone_id = self._get_zone_id(context, client, dns_domain) + recordsets = client.recordsets.list( + zone_id, criterion={'name': fqdn, 'type': 'A'}) + for rs in recordsets: + client.recordsets.delete(zone_id, rs['id']) + LOG.info("Deleted DNS A record '%s' (recordset %s)", + fqdn, rs['id']) + return True + except Exception: + LOG.exception("Failed to delete DNS A record '%s'. " + "Manual cleanup may be required.", fqdn) + return False + + def update_record(self, context, dns_name, dns_domain, ip_addresses): + """Update a DNS A recordset in Designate.""" + + if not self.enabled: + return True + + fqdn = '%s.%s' % (dns_name, dns_domain) + try: + client = self._get_client(context) + zone_id = self._get_zone_id(context, client, dns_domain) + recordsets = client.recordsets.list( + zone_id, criterion={'name': fqdn, 'type': 'A'}) + found = False + for rs in recordsets: + client.recordsets.update( + zone_id, rs['id'], + records=ip_addresses) + LOG.info("Updated DNS A record '%s' -> %s", + fqdn, ip_addresses) + found = True + break + if not found: + return self.create_record( + context, dns_name, dns_domain, ip_addresses) + return True + except exception.NotFound: + LOG.warning("Zone not found while updating '%s'; " + "refreshing zone cache and retrying.", fqdn) + self._zone_id_cache.pop((context.project_id, dns_domain), None) + try: + self.create_record(context, dns_name, dns_domain, ip_addresses) + return True + except Exception: + LOG.exception("Failed to update DNS A record '%s' after " + "zone cache refresh.", fqdn) + return False + except Exception: + LOG.exception("Failed to update DNS A record '%s'.", fqdn) + return False diff --git a/manila/share/api.py b/manila/share/api.py index 31f3642b8b..a6ece58c83 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -132,6 +132,44 @@ def locked_share_server_allocations_operation(*_args, **_kwargs): return wrapped +def _validate_dns_metadata(metadata): + """Validate dns_name and dns_domain share metadata values. + + Normalizes ``dns_domain`` by appending a trailing dot if absent. + Mutates the *metadata* dict in-place so callers receive the canonical + form without having to do extra work. + """ + + if not metadata: + return + dns_name = metadata.get('dns_name') + dns_domain = metadata.get('dns_domain') + if dns_name and not dns_domain: + raise exception.InvalidInput( + reason=_("'dns_domain' is required when 'dns_name' is set.")) + if dns_domain and not dns_name: + raise exception.InvalidInput( + reason=_("'dns_name' is required when 'dns_domain' is set.")) + if dns_name: + if '.' in dns_name: + raise exception.InvalidInput( + reason=_("'dns_name' must be a single DNS label and must " + "not contain dots. Got: '%s'.") % dns_name) + # RFC 1035 label: 1-63 chars, [a-zA-Z0-9-], no leading/trailing '-' + if not re.match(r'^[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?$', + dns_name): + raise exception.InvalidInput( + reason=_("'dns_name' must be a valid RFC 1035 DNS label: " + "1-63 characters, alphanumeric and hyphens only, " + "not starting or ending with a hyphen. " + "Got: '%s'.") % dns_name) + if dns_domain: + # Normalize: silently append the trailing dot for user convenience. + if not dns_domain.endswith('.'): + dns_domain = dns_domain + '.' + metadata['dns_domain'] = dns_domain + + class API(base.Base): """API for interacting with the share manager.""" @@ -309,6 +347,7 @@ def create(self, context, share_proto, size, name, description, """Create new share.""" api_common.check_metadata_properties(metadata) + _validate_dns_metadata(metadata) if snapshot_id is not None: snapshot = self.get_snapshot(context, snapshot_id) @@ -598,19 +637,31 @@ def update_metadata_from_share_type_extra_specs(self, context, share_type, return metadata_from_share_type def update_share_from_metadata(self, context, share_id, metadata): - driver_keys = getattr(CONF, 'driver_updatable_metadata', []) - if not driver_keys: - return + driver_keys = set(getattr(CONF, 'driver_updatable_metadata', [])) + dns_keys = {'dns_name', 'dns_domain'} - driver_metadata = {} - for k, v in metadata.items(): - if k in driver_keys: - driver_metadata.update({k: v}) + relevant_metadata = {k: v for k, v in metadata.items() + if k in driver_keys or k in dns_keys} - if driver_metadata: - share = self.get(context, share_id) - self.share_rpcapi.update_share_from_metadata(context, share, - driver_metadata) + if not relevant_metadata: + return + + share = self.get(context, share_id) + + has_dns_keys = bool( + relevant_metadata.keys() & dns_keys) + if has_dns_keys: + if (share['status'] != constants.STATUS_AVAILABLE + or share.get('task_state') is not None): + raise exception.InvalidShare( + reason=_("DNS metadata can only be updated when the " + "share is in 'available' status with no " + "pending task. Current status: '%s', " + "task_state: '%s'.") + % (share['status'], share.get('task_state'))) + + self.share_rpcapi.update_share_from_metadata(context, share, + relevant_metadata) def update_share_network_subnet_from_metadata(self, context, share_network_id, diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py index 9d8d0376c1..9ac587338a 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -2459,11 +2459,13 @@ def _get_export_addresses_with_metadata(self, share, share_server, preferred = interface.get('home-node') in home_node_set - addresses[address] = { + metadata = { 'is_admin_only': is_admin_only, 'preferred': preferred, } + addresses[address] = metadata + return addresses @na_utils.trace diff --git a/manila/share/manager.py b/manila/share/manager.py index ade8268d13..9661fd8eef 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -42,6 +42,7 @@ from manila import context from manila import coordination from manila.data import rpcapi as data_rpcapi +from manila.dns import designate as dns_designate from manila import exception from manila.i18n import _ from manila.keymgr import barbican as barbican_api @@ -385,6 +386,7 @@ def __init__(self, share_driver=None, service_name=None, *args, **kwargs): self.message_api = message_api.API() self.share_api = api.API() self.transfer_api = transfer_api.API() + self.dns_api = dns_designate.API() if CONF.profiler.enabled and profiler is not None: self.driver = profiler.trace_cls("driver")(self.driver) self.hooks = [] @@ -2056,6 +2058,9 @@ def _migration_complete_driver( self.db.export_locations_update( context, dest_share_instance['id'], data_updates['export_locations']) + self._update_dns_record_on_migration( + context, share_ref, dest_share_instance, + data_updates['export_locations']) snapshot_updates = data_updates.get('snapshot_updates') or {} @@ -2412,6 +2417,147 @@ def _get_share_instance(self, context, share): id = share.instance['id'] return self.db.share_instance_get(context, id, with_share_data=True) + def _create_dns_export_locations(self, dns_name, dns_domain, + export_locations): + """Creates DNS-based export locations if DNS metadata is present.""" + dns_locations = [] + + if not dns_name or not dns_domain: + return dns_locations + + for export_loc in export_locations: + if not export_loc.get('metadata', {}).get('preferred'): + continue + + ip_path = export_loc['path'] + try: + mountpoint = ip_path.split(':')[1] + except IndexError: + LOG.warning("Invalid export location path: %s", ip_path) + continue + + dns_path = f"{dns_name}.{dns_domain.rstrip('.')}:{mountpoint}" + + # DNS-based location is preferred for easy consumption; + # the IP-based location is kept but marked non-preferred. + dns_location = { + 'path': dns_path, + 'is_admin_only': False, + 'metadata': { + 'preferred': True, + 'dns_name': dns_name, + 'dns_domain': dns_domain + } + } + dns_locations.append(dns_location) + break + + return dns_locations + + def _demote_ip_export_locations(self, export_locations): + """Return export locations with preferred=False on IP-based entries. + + Called when DNS export locations are present so that DNS paths become + the single preferred mount point for clients. + """ + result = [] + for loc in export_locations: + meta = dict(loc.get('metadata', {})) + if meta.get('preferred'): + meta['preferred'] = False + result.append(dict(loc, metadata=meta)) + return result + + def _restore_ip_preferred(self, export_locations): + """Restore preferred=True on the first non-admin IP export location. + + Called when DNS metadata is removed so that clients fall back to + the IP-based path as their preferred mount point. + """ + result = [] + restored = False + for loc in export_locations: + meta = dict(loc.get('metadata', {})) + if not restored and not loc.get('is_admin_only'): + meta['preferred'] = True + restored = True + result.append(dict(loc, metadata=meta)) + return result + + def _get_preferred_ips(self, export_locations): + """Extract IPs from preferred, non-admin export locations.""" + + ips = [] + for loc in export_locations: + if loc.get('is_admin_only'): + continue + if not loc.get('metadata', {}).get('preferred'): + continue + ip = loc.get('path', '').split(':')[0] + if ip: + ips.append(ip) + return ips + + def _create_dns_record(self, context, share_instance, dns_name, dns_domain, + export_locations): + """Creates a DNS A record in Designate for the preferred export IP.""" + + if not self.dns_api.enabled: + return + + if not dns_name or not dns_domain: + return + + ip_addresses = self._get_preferred_ips(export_locations) + + if not ip_addresses: + LOG.warning("No preferred export location IP found for share " + "instance %s, skipping DNS record creation.", + share_instance['id']) + return + + self.dns_api.create_record(context, dns_name, dns_domain, ip_addresses) + + def _delete_dns_record(self, context, share): + """Deletes the DNS A record in Designate for a share.""" + if not self.dns_api.enabled: + return + + share_metadata = self.db.share_metadata_get(context, share['id']) + dns_name = share_metadata.get('dns_name') + dns_domain = share_metadata.get('dns_domain') + + if not dns_name or not dns_domain: + return + + self.dns_api.delete_record(context, dns_name, dns_domain) + + def _update_dns_record_on_migration(self, context, share_ref, + dest_share_instance, export_locations): + """Update Designate DNS A record after share migration completes.""" + + if not self.dns_api.enabled: + return + + share_metadata = self.db.share_metadata_get(context, share_ref['id']) + dns_name = share_metadata.get('dns_name') + dns_domain = share_metadata.get('dns_domain') + + if not dns_name or not dns_domain: + return + + ip_addresses = self._get_preferred_ips(export_locations) + + if not ip_addresses: + LOG.warning("No preferred export location IP found after " + "migration of share instance %s, " + "skipping DNS record update.", + dest_share_instance['id']) + return + + self.dns_api.update_record( + context, dns_name, dns_domain, ip_addresses) + @run_concurrently @add_hooks @utils.require_driver_initialized @@ -2677,6 +2823,25 @@ def create_share_instance(self, context, share_instance_id, self.db.export_locations_update( context, share_instance['id'], export_locations) + _share_meta = self.db.share_metadata_get( + context, share_instance['share_id']) + _dns_name = _share_meta.get('dns_name') + _dns_domain = _share_meta.get('dns_domain') + + dns_locations = self._create_dns_export_locations( + _dns_name, _dns_domain, export_locations) + + if dns_locations: + self.db.export_locations_update( + context, share_instance['id'], + self._demote_ip_export_locations( + export_locations) + dns_locations, + delete=True) + + self._create_dns_record( + context, share_instance, _dns_name, _dns_domain, + export_locations) + except Exception as e: with excutils.save_and_reraise_exception(): LOG.warning("Share instance %s failed on creation.", @@ -2710,6 +2875,23 @@ def get_export_location(details): resource_type=message_field.Resource.SHARE, resource_id=share_id, exception=e) + # Clean up any Designate record that was written before the + # failure so we do not leave orphaned records behind. + _share_meta_cleanup = self.db.share_metadata_get( + context, share_id) + _cleanup_dns_name = _share_meta_cleanup.get('dns_name') + _cleanup_dns_domain = _share_meta_cleanup.get('dns_domain') + if _cleanup_dns_name and _cleanup_dns_domain: + try: + self.dns_api.delete_record( + context, _cleanup_dns_name, _cleanup_dns_domain) + except Exception: + LOG.warning( + "Could not clean up DNS record '%s.%s' after " + "share instance %s creation failure. " + "Manual cleanup in Designate may be required.", + _cleanup_dns_name, _cleanup_dns_domain, + share_instance_id) else: LOG.info("Share instance %s created successfully.", share_instance_id) @@ -3111,6 +3293,8 @@ def promote_share_replica(self, context, share_replica_id, share_id=None, share_metadata = { m['key']: m['value'] for m in share_metadata_list } if share_metadata_list else {} + dns_name = share_metadata.get('dns_name') + dns_domain = share_metadata.get('dns_domain') for r in replica_list: replica_metadata = self.db.share_replica_metadata_get( @@ -3234,6 +3418,45 @@ def promote_share_replica(self, context, share_replica_id, share_id=None, LOG.info("Share replica %s: promoted to active state " "successfully.", share_replica['id']) + if self.dns_api.enabled and dns_name and dns_domain: + new_export_locs = ( + self.db.export_location_get_all_by_share_instance_id( + context, share_replica['id'])) + ip_locs = [ + { + 'path': el['path'], + 'is_admin_only': el['is_admin_only'], + 'metadata': el.get('el_metadata', {}), + } + for el in new_export_locs + if not el.get('el_metadata', {}).get('dns_name') + ] + ip_addresses = self._get_preferred_ips(ip_locs) + if ip_addresses: + # Update the Designate record to the new IP. + self.dns_api.update_record( + context, dns_name, dns_domain, ip_addresses) + # Rebuild DNS export locations on this replica in case it + # was created before DNS metadata was set. + existing_dns_locs = [ + el for el in new_export_locs + if el.get('el_metadata', {}).get('dns_name') + ] + if not existing_dns_locs: + dns_locations = self._create_dns_export_locations( + dns_name, dns_domain, ip_locs) + if dns_locations: + self.db.export_locations_update( + context, share_replica['id'], + self._demote_ip_export_locations( + ip_locs) + dns_locations, + delete=True) + else: + LOG.warning( + "No preferred export location IP found on promoted " + "replica %s; DNS A record was not updated.", + share_replica['id']) + @periodic_task.periodic_task(spacing=CONF.replica_state_update_interval) @utils.require_driver_initialized def periodic_share_replica_update(self, context): @@ -4087,6 +4310,18 @@ def delete_share_instance(self, context, share_instance_id, force=False, self._notify_about_share_usage(context, share, share_instance, "delete.start") + share_metadata = self.db.share_metadata_get(context, share['id']) + dns_name = share_metadata.get('dns_name') + dns_domain = share_metadata.get('dns_domain') + if dns_name and dns_domain: + other_instances = [ + si for si in self.db.share_instance_get_all_by_share( + context, share['id']) + if si['id'] != share_instance_id + ] + if not other_instances: + self._delete_dns_record(context, share) + error_state = None if deferred_delete: try: @@ -7594,6 +7829,89 @@ def update_share_server_network_allocations( self._check_share_network_update_finished( context, share_network_id=share_network['id']) + def _sync_dns_on_metadata_update(self, context, share, share_instance): + """Sync Designate DNS record and DNS export locations. + + Called after a metadata update on the share. + """ + + share_metadata = self.db.share_metadata_get(context, share['id']) + dns_name = share_metadata.get('dns_name') + dns_domain = share_metadata.get('dns_domain') + + # Fetch current export locations from DB (includes DNS ones) + existing_els = self.db.export_location_get_all_by_share_instance_id( + context, share_instance['id']) + + ip_export_locs = [] + dns_export_locs = [] + for el in existing_els: + el_meta = el.get('el_metadata', {}) + if el_meta.get('dns_name'): + dns_export_locs.append(el) + else: + ip_export_locs.append({ + 'path': el['path'], + 'is_admin_only': el['is_admin_only'], + 'metadata': el_meta, + }) + + # Detect the old FQDN stored in the existing DNS export locations + old_dns_name = None + old_dns_domain = None + for el in dns_export_locs: + el_meta = el.get('el_metadata', {}) + old_dns_name = el_meta.get('dns_name') + old_dns_domain = el_meta.get('dns_domain') + break + + if dns_name and dns_domain: + fqdn_changed = (old_dns_name and ( + old_dns_name != dns_name or old_dns_domain != dns_domain)) + + if fqdn_changed: + # Remove old DNS export locations and Designate record before + # writing the new ones, to avoid orphaned records. + restored_ip_locs = self._restore_ip_preferred(ip_export_locs) + self.db.export_locations_update( + context, share_instance['id'], + restored_ip_locs, delete=True) + self.dns_api.delete_record( + context, old_dns_name, old_dns_domain) + dns_export_locs = [] + + new_dns_locs = self._create_dns_export_locations( + dns_name, dns_domain, ip_export_locs) + if new_dns_locs and not dns_export_locs: + self.db.export_locations_update( + context, share_instance['id'], + self._demote_ip_export_locations( + ip_export_locs) + new_dns_locs, + delete=True) + self._create_dns_record( + context, share_instance, dns_name, dns_domain, ip_export_locs) + else: + # DNS metadata removed: clean up DNS export locations and A-record. + # Pass only the IP-based locations with delete=True so that the DB + # layer soft-deletes any export locations not in the provided list. + # Also restore preferred=True on the IP locations since DNS + # is gone. + if dns_export_locs: + restored_ip_locs = self._restore_ip_preferred(ip_export_locs) + self.db.export_locations_update( + context, share_instance['id'], + restored_ip_locs, delete=True) + # Recover FQDN from stored DNS export location metadata and delete + # the corresponding Designate record. + for el in dns_export_locs: + el_meta = el.get('el_metadata', {}) + old_dns_name = el_meta.get('dns_name') + old_dns_domain = el_meta.get('dns_domain') + if old_dns_name and old_dns_domain: + self.dns_api.delete_record( + context, old_dns_name, old_dns_domain) + break + def update_share_from_metadata(self, context, share_id, metadata): share = self.db.share_get(context, share_id) share_instance = self._get_share_instance(context, share) @@ -7617,6 +7935,8 @@ def update_share_from_metadata(self, context, share_id, metadata): resource_id=share_id, detail=message_field.Detail.UPDATE_METADATA_FAILURE) + self._sync_dns_on_metadata_update(context, share, share_instance) + def update_share_network_subnet_from_metadata(self, context, share_network_id, share_network_subnet_id, diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index a7cbcee5da..9f38ddfc3b 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -2359,11 +2359,20 @@ def test_update_all_metadata(self): def test_delete_metadata(self): mock_delete = self.mock_object( self.controller, '_delete_metadata', mock.Mock()) + mock_update = self.mock_object( + self.controller.share_api, 'update_share_from_metadata', + mock.Mock()) + remaining_metadata = {'test_key': 'test_value'} + self.mock_object( + self.controller.share_api.db, 'share_metadata_get', + mock.Mock(return_value=remaining_metadata)) req = fakes.HTTPRequest.blank( '/v2/shares/%s/metadata/fake_key' % id) self.controller.delete_metadata(req, id, 'fake_key') mock_delete.assert_called_once_with(req, id, 'fake_key') + mock_update.assert_called_once_with( + req.environ['manila.context'], id, remaining_metadata) def _fake_access_get(self, ctxt, access_id): diff --git a/manila/tests/api/views/test_export_locations_dns.py b/manila/tests/api/views/test_export_locations_dns.py new file mode 100644 index 0000000000..5f48b4ed0b --- /dev/null +++ b/manila/tests/api/views/test_export_locations_dns.py @@ -0,0 +1,125 @@ +# Copyright 2026 SAP SE. +# 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 unittest import mock + +from manila.api.openstack import api_version_request +from manila.api.views import export_locations +from manila import test + + +class ExportLocationsViewBuilderDNSTestCase(test.TestCase): + def setUp(self): + super(ExportLocationsViewBuilderDNSTestCase, self).setUp() + self.view_builder = export_locations.ViewBuilder() + + def _get_mock_export_location(self, dns_name=None, dns_domain=None, + preferred=False): + el_metadata = {'preferred': str(preferred).lower()} + if dns_name: + el_metadata['dns_name'] = dns_name + if dns_domain: + el_metadata['dns_domain'] = dns_domain + + return { + 'uuid': 'location-id-123', + 'path': '192.168.1.50:/share', + 'is_admin_only': False, + 'share_instance_id': 'instance-id-123', + 'created_at': '2026-04-09T10:00:00Z', + 'updated_at': '2026-04-09T10:00:00Z', + 'el_metadata': el_metadata, + } + + def _get_request(self, version, is_admin=False): + context = mock.Mock() + context.is_admin = is_admin + request = mock.Mock() + request.environ = {'manila.context': context} + request.api_version_request = api_version_request.APIVersionRequest( + version) + return request + + def test_dns_metadata_added_to_export_location_view(self): + export_location = self._get_mock_export_location( + dns_name='my_pet_share', + dns_domain='manila.com.') + + request = self._get_request('2.96', is_admin=True) + view = self.view_builder.summary(request, export_location) + view_dict = view['export_location'] + + self.assertEqual(view_dict['dns_name'], 'my_pet_share') + self.assertEqual(view_dict['dns_domain'], 'manila.com.') + + def test_dns_metadata_not_added_when_missing(self): + export_location = self._get_mock_export_location() + + request = self._get_request('2.96') + view = self.view_builder.summary(request, export_location) + view_dict = view['export_location'] + + self.assertNotIn('dns_name', view_dict) + self.assertNotIn('dns_domain', view_dict) + + def test_partial_dns_metadata(self): + """Test with only dns_name (no dns_domain).""" + export_location = self._get_mock_export_location( + dns_name='test_share') + + request = self._get_request('2.96', is_admin=True) + view = self.view_builder.summary(request, export_location) + view_dict = view['export_location'] + + self.assertEqual(view_dict['dns_name'], 'test_share') + self.assertNotIn('dns_domain', view_dict) + + def test_dns_domain_only(self): + """Test with only dns_domain (no dns_name).""" + export_location = self._get_mock_export_location( + dns_domain='example.com.') + + request = self._get_request('2.96', is_admin=True) + view = self.view_builder.summary(request, export_location) + view_dict = view['export_location'] + + self.assertEqual(view_dict['dns_domain'], 'example.com.') + self.assertNotIn('dns_name', view_dict) + + def test_dns_metadata_preserved_with_preferred(self): + export_location = self._get_mock_export_location( + dns_name='share1', + dns_domain='test.com.', + preferred=True) + + request = self._get_request('2.96', is_admin=True) + view = self.view_builder.summary(request, export_location) + view_dict = view['export_location'] + + self.assertTrue(view_dict['preferred']) + self.assertEqual(view_dict['dns_name'], 'share1') + self.assertEqual(view_dict['dns_domain'], 'test.com.') + + def test_dns_fields_absent_before_2_96(self): + export_location = self._get_mock_export_location( + dns_name='share1', + dns_domain='test.com.') + + request = self._get_request('2.95', is_admin=True) + view = self.view_builder.summary(request, export_location) + view_dict = view['export_location'] + + self.assertNotIn('dns_name', view_dict) + self.assertNotIn('dns_domain', view_dict) diff --git a/manila/tests/dns/__init__.py b/manila/tests/dns/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/manila/tests/dns/test_designate_integration.py b/manila/tests/dns/test_designate_integration.py new file mode 100644 index 0000000000..55faee90f6 --- /dev/null +++ b/manila/tests/dns/test_designate_integration.py @@ -0,0 +1,229 @@ +# Copyright 2026 SAP SE. +# 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 unittest import mock + +from manila.dns import designate +from manila import exception +from manila import test + + +class DesignateAPIIntegrationTestCase(test.TestCase): + def setUp(self): + super(DesignateAPIIntegrationTestCase, self).setUp() + self.context = mock.Mock() + self.context.is_admin = True + + @mock.patch('manila.dns.designate.CONF') + def test_designate_api_enabled_when_configured(self, mock_conf): + mock_conf.__getitem__.return_value.enabled = True + + api = designate.API() + self.assertTrue(api.enabled) + + @mock.patch('manila.dns.designate.CONF') + def test_designate_api_disabled_when_not_enabled(self, mock_conf): + mock_conf.__getitem__.return_value.enabled = False + + api = designate.API() + self.assertFalse(api.enabled) + + @mock.patch('manila.dns.designate.designateclient') + @mock.patch('manila.dns.designate.CONF') + def test_create_dns_record_success(self, mock_conf, mock_designate_client): + mock_conf.__getitem__.return_value.enabled = True + mock_conf.__getitem__.return_value.ttl = 300 + + mock_client = mock.Mock() + mock_designate_client.return_value = mock_client + mock_client.zones.list.return_value = [ + {'id': '9cd5947b-9173-488a-940f-57eeeb7604a3', + 'name': 'user_test.com.'} + ] + mock_client.recordsets.create.return_value = { + 'id': 'recordset-123', + 'name': 'test-share.user_test.com.', + 'type': 'A', + 'records': ['192.168.1.50'] + } + self.context.project_id = None + + api = designate.API() + result = api.create_record( + self.context, + dns_name='test-share', + dns_domain='user_test.com.', + ip_addresses=['192.168.1.50'] + ) + + self.assertEqual(result, 'recordset-123') + mock_client.zones.list.assert_called_once_with( + criterion={'name': 'user_test.com.'}) + mock_client.recordsets.create.assert_called_once() + + @mock.patch('manila.dns.designate.designateclient') + @mock.patch('manila.dns.designate.CONF') + def test_create_dns_record_zone_not_found(self, mock_conf, + mock_designate_client): + mock_conf.__getitem__.return_value.enabled = True + mock_conf.__getitem__.return_value.ttl = 300 + + mock_client = mock.Mock() + mock_designate_client.return_value = mock_client + mock_client.zones.list.return_value = [] + + api = designate.API() + self.assertRaises( + exception.NotFound, + api.create_record, + self.context, + dns_name='test-share', + dns_domain='unknown.com.', + ip_addresses=['192.168.1.50'], + ) + mock_client.recordsets.create.assert_not_called() + + @mock.patch('manila.dns.designate.designateclient') + @mock.patch('manila.dns.designate.CONF') + def test_create_dns_record_failure_raises(self, mock_conf, + mock_designate_client): + mock_conf.__getitem__.return_value.enabled = True + mock_conf.__getitem__.return_value.ttl = 300 + + mock_client = mock.Mock() + mock_designate_client.return_value = mock_client + mock_client.zones.list.return_value = [ + {'id': '9cd5947b-9173-488a-940f-57eeeb7604a3', + 'name': 'user_test.com.'} + ] + mock_client.recordsets.create.side_effect = RuntimeError( + "Designate API error") + + api = designate.API() + self.assertRaises( + RuntimeError, + api.create_record, + self.context, + dns_name='test-share', + dns_domain='user_test.com.', + ip_addresses=['192.168.1.50'], + ) + + @mock.patch('manila.dns.designate.designateclient') + @mock.patch('manila.dns.designate.CONF') + def test_delete_dns_record_success(self, mock_conf, + mock_designate_client): + """Test successful DNS record deletion.""" + mock_conf.__getitem__.return_value.enabled = True + + mock_client = mock.Mock() + mock_designate_client.return_value = mock_client + mock_client.zones.list.return_value = [ + {'id': '9cd5947b-9173-488a-940f-57eeeb7604a3', + 'name': 'user_test.com.'} + ] + mock_client.recordsets.list.return_value = [ + {'id': 'recordset-123', 'name': 'test-share.user_test.com.'} + ] + + api = designate.API() + result = api.delete_record( + self.context, + dns_name='test-share', + dns_domain='user_test.com.' + ) + + self.assertTrue(result) + mock_client.recordsets.list.assert_called_once() + mock_client.recordsets.delete.assert_called_once() + + @mock.patch('manila.dns.designate.designateclient') + @mock.patch('manila.dns.designate.CONF') + def test_update_dns_record_success(self, mock_conf, + mock_designate_client): + mock_conf.__getitem__.return_value.enabled = True + + mock_client = mock.Mock() + mock_designate_client.return_value = mock_client + mock_client.zones.list.return_value = [ + {'id': '9cd5947b-9173-488a-940f-57eeeb7604a3', + 'name': 'user_test.com.'} + ] + mock_client.recordsets.list.return_value = [ + {'id': 'recordset-123', 'name': 'test-share.user_test.com.'} + ] + + api = designate.API() + result = api.update_record( + self.context, + dns_name='test-share', + dns_domain='user_test.com.', + ip_addresses=['192.168.1.100'] + ) + + self.assertTrue(result) + mock_client.recordsets.update.assert_called_once() + + @mock.patch('manila.dns.designate.designateclient') + @mock.patch('manila.dns.designate.CONF') + def test_update_dns_record_creates_if_not_exists(self, mock_conf, + mock_designate_client): + mock_conf.__getitem__.return_value.enabled = True + mock_conf.__getitem__.return_value.ttl = 300 + + mock_client = mock.Mock() + mock_designate_client.return_value = mock_client + mock_client.zones.list.return_value = [ + {'id': '9cd5947b-9173-488a-940f-57eeeb7604a3', + 'name': 'user_test.com.'} + ] + mock_client.recordsets.list.return_value = [] + mock_client.recordsets.create.return_value = { + 'id': 'recordset-456', + 'name': 'new-share.user_test.com.', + 'type': 'A', + 'records': ['192.168.1.200'] + } + + api = designate.API() + result = api.update_record( + self.context, + dns_name='new-share', + dns_domain='user_test.com.', + ip_addresses=['192.168.1.200'] + ) + + self.assertEqual(result, 'recordset-456') + mock_client.recordsets.create.assert_called_once() + + @mock.patch('manila.dns.designate.designateclient') + @mock.patch('manila.dns.designate.CONF') + def test_get_zone_id_appends_dot_if_missing(self, mock_conf, + mock_designate_client): + """_get_zone_id normalises domain to trailing dot before lookup.""" + mock_conf.__getitem__.return_value.enabled = True + + mock_client = mock.Mock() + mock_client.zones.list.return_value = [ + {'id': 'zone-abc', 'name': 'example.com.'} + ] + + api = designate.API() + zone_id = api._get_zone_id(self.context, mock_client, 'example.com') + + self.assertEqual(zone_id, 'zone-abc') + mock_client.zones.list.assert_called_once_with( + criterion={'name': 'example.com.', + 'project_id': self.context.project_id}) diff --git a/manila/tests/dns/test_designate_scenarios.py b/manila/tests/dns/test_designate_scenarios.py new file mode 100644 index 0000000000..bab34a4406 --- /dev/null +++ b/manila/tests/dns/test_designate_scenarios.py @@ -0,0 +1,179 @@ +# Copyright 2026 SAP SE. +# 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 unittest import mock + +from oslo_config import cfg + +from manila.dns import designate +from manila import test + + +CONF = cfg.CONF + + +class DesignateShareLifecycleTestCase(test.TestCase): + def setUp(self): + super(DesignateShareLifecycleTestCase, self).setUp() + self.context = mock.Mock() + self.context.is_admin = True + self.zone_id = '9cd5947b-9173-488a-940f-57eeeb7604a3' + self.dns_name = 'test-share' + self.dns_domain = 'user_test.com.' + self.ip_addresses = ['192.168.1.50'] + + def _make_client_mock(self, zone_id=None): + """Return a mock Designate client with zones.list pre-configured.""" + mock_client = mock.Mock() + mock_client.zones.list.return_value = [ + {'id': zone_id or self.zone_id, 'name': self.dns_domain} + ] + return mock_client + + @mock.patch('manila.dns.designate.designateclient') + @mock.patch('manila.dns.designate.CONF') + def test_share_lifecycle_with_dns(self, mock_conf, mock_designate_client): + mock_conf.__getitem__.return_value.enabled = True + mock_conf.__getitem__.return_value.ttl = 300 + + mock_client = self._make_client_mock() + mock_designate_client.return_value = mock_client + + mock_client.recordsets.create.return_value = { + 'id': 'recordset-1', + 'name': f'{self.dns_name}.{self.dns_domain}', + 'type': 'A', + 'records': self.ip_addresses + } + + api = designate.API() + recordset_id = api.create_record( + self.context, + self.dns_name, + self.dns_domain, + self.ip_addresses + ) + self.assertEqual(recordset_id, 'recordset-1') + + new_ip = ['192.168.1.100'] + mock_client.recordsets.list.return_value = [ + {'id': 'recordset-1', 'name': f'{self.dns_name}.{self.dns_domain}'} + ] + + update_result = api.update_record( + self.context, + self.dns_name, + self.dns_domain, + new_ip + ) + self.assertTrue(update_result) + mock_client.recordsets.update.assert_called_once() + + delete_result = api.delete_record( + self.context, + self.dns_name, + self.dns_domain + ) + self.assertTrue(delete_result) + mock_client.recordsets.delete.assert_called_once() + + @mock.patch('manila.dns.designate.designateclient') + @mock.patch('manila.dns.designate.CONF') + def test_multiple_ips_for_single_share(self, mock_conf, + mock_designate_client): + mock_conf.__getitem__.return_value.enabled = True + mock_conf.__getitem__.return_value.ttl = 300 + + mock_client = self._make_client_mock() + mock_designate_client.return_value = mock_client + + multiple_ips = ['192.168.1.50', '192.168.1.51', '192.168.1.52'] + + mock_client.recordsets.create.return_value = { + 'id': 'recordset-multi', + 'name': f'{self.dns_name}.{self.dns_domain}', + 'type': 'A', + 'records': multiple_ips + } + + api = designate.API() + recordset_id = api.create_record( + self.context, + self.dns_name, + self.dns_domain, + multiple_ips + ) + + self.assertEqual(recordset_id, 'recordset-multi') + call_args = mock_client.recordsets.create.call_args + self.assertEqual(call_args[0][3], multiple_ips) + + @mock.patch('manila.dns.designate.CONF') + def test_api_disabled_with_missing_config(self, mock_conf): + mock_conf.__getitem__.return_value.enabled = False + + api = designate.API() + self.assertFalse(api.enabled) + + result = api.create_record( + self.context, + self.dns_name, + self.dns_domain, + self.ip_addresses + ) + self.assertIsNone(result) + + result = api.delete_record( + self.context, + self.dns_name, + self.dns_domain + ) + self.assertTrue(result) + + @mock.patch('manila.dns.designate.designateclient') + @mock.patch('manila.dns.designate.CONF') + def test_different_domains_use_different_zones(self, mock_conf, + mock_designate_client): + """Each dns_domain resolves its own zone independently.""" + mock_conf.__getitem__.return_value.enabled = True + mock_conf.__getitem__.return_value.ttl = 300 + + mock_client = mock.Mock() + mock_designate_client.return_value = mock_client + + zone_map = { + 'team-a.example.com.': 'zone-aaa', + 'team-b.internal.': 'zone-bbb', + } + + def zones_list(criterion): + name = criterion['name'] + zid = zone_map.get(name) + return [{'id': zid, 'name': name}] if zid else [] + + mock_client.zones.list.side_effect = zones_list + mock_client.recordsets.create.side_effect = lambda zid, *a, **kw: { + 'id': 'rs-' + zid} + + api = designate.API() + + r1 = api.create_record(self.context, 'share1', + 'team-a.example.com.', ['10.0.0.1']) + r2 = api.create_record(self.context, 'share2', + 'team-b.internal.', ['10.0.0.2']) + + self.assertEqual(r1, 'rs-zone-aaa') + self.assertEqual(r2, 'rs-zone-bbb') + self.assertEqual(mock_client.zones.list.call_count, 2) diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base_dns.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base_dns.py new file mode 100644 index 0000000000..62de0446ea --- /dev/null +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base_dns.py @@ -0,0 +1,107 @@ +# Copyright 2026 SAP SE. +# 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 unittest import mock + +from manila import context +from manila.share.drivers.netapp.dataontap.cluster_mode import lib_base +from manila import test + + +class NetAppLibBaseDNSMetadataTestCase(test.TestCase): + """Tests that NetApp driver export addresses do NOT contain DNS metadata. + + DNS handling is the responsibility of ShareManager, not the driver. + """ + + def setUp(self): + super(NetAppLibBaseDNSMetadataTestCase, self).setUp() + self.context = context.get_admin_context() + + def _get_mock_driver(self): + driver = mock.MagicMock( + spec=lib_base.NetAppCmodeFileStorageLibrary) + driver._is_flexgroup_pool = mock.Mock(return_value=False) + driver._get_aggregate_node = mock.Mock(return_value='node1') + driver._get_admin_addresses_for_share_server = mock.Mock( + return_value=['10.0.0.1']) + return driver + + def _get_mock_interfaces(self): + return [ + { + 'address': '192.168.1.50', + 'home-node': 'node1', + }, + { + 'address': '192.168.1.51', + 'home-node': 'node2', + }, + ] + + def _get_mock_share(self, dns_name=None, dns_domain=None): + metadata = {} + if dns_name: + metadata['dns_name'] = dns_name + if dns_domain: + metadata['dns_domain'] = dns_domain + + return { + 'id': 'share-id-123', + 'host': 'backend@pool1', + 'metadata': metadata, + } + + def _call_method(self, share, share_server=None): + driver = self._get_mock_driver() + interfaces = self._get_mock_interfaces() + method = lib_base.NetAppCmodeFileStorageLibrary\ + ._get_export_addresses_with_metadata + return method(driver, share, share_server, interfaces, share['host']) + + def test_export_addresses_no_dns_metadata_without_share_dns(self): + addresses = self._call_method(self._get_mock_share()) + + for address, metadata in addresses.items(): + self.assertNotIn('dns_name', metadata) + self.assertNotIn('dns_domain', metadata) + self.assertIn('preferred', metadata) + self.assertIn('is_admin_only', metadata) + + def test_export_addresses_no_dns_metadata_with_share_dns(self): + share = self._get_mock_share( + dns_name='my_pet_share', dns_domain='manila.com.') + addresses = self._call_method(share) + + for address, metadata in addresses.items(): + self.assertNotIn('dns_name', metadata) + self.assertNotIn('dns_domain', metadata) + + def test_preferred_path_identified_correctly(self): + share = self._get_mock_share() + addresses = self._call_method(share) + + self.assertTrue(addresses['192.168.1.50']['preferred']) + self.assertFalse(addresses['192.168.1.51']['preferred']) + + def test_metadata_structure(self): + addresses = self._call_method(self._get_mock_share()) + + for address, metadata in addresses.items(): + self.assertIsInstance(metadata, dict) + self.assertIn('is_admin_only', metadata) + self.assertIn('preferred', metadata) + self.assertIsInstance(metadata['is_admin_only'], bool) + self.assertIsInstance(metadata['preferred'], bool) diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index f2d082efb3..cabd12a6d6 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -732,10 +732,11 @@ def test_update_share_from_metadata(self): 'snapshot_policy': 'monthly', 'max_share_size': '10' } - backend_metadata = { - k: v for k, v in metadata.items() if k != 'max_share_size'} + expected_metadata = {'dedupe': 'True', 'snapshot_policy': 'monthly'} self.mock_object(self.api, 'get', mock.Mock(return_value='fake_share')) + self.mock_object( + self.api.db, 'share_metadata_get', mock.Mock(return_value={})) mock_call = self.mock_object( self.api.share_rpcapi, 'update_share_from_metadata' @@ -743,7 +744,7 @@ def test_update_share_from_metadata(self): self.api.update_share_from_metadata(self.context, 'fake_id', metadata) mock_call.assert_called_once_with( - self.context, 'fake_share', backend_metadata) + self.context, 'fake_share', expected_metadata) @ddt.data(True, False) def test_create_public_and_private_share(self, is_public): diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index 1d73c0b521..70ab3f1e39 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -11681,3 +11681,170 @@ def test_hooks_disabled(self): for mock_hook in self.hooks: self.assertFalse(mock_hook.execute_pre_hook.called) self.assertFalse(mock_hook.execute_post_hook.called) + + +class CreateDnsExportLocationsTestCase(test.TestCase): + + def setUp(self): + super(CreateDnsExportLocationsTestCase, self).setUp() + self.flags(share_driver='manila.tests.fake_driver.FakeShareDriver') + self.share_manager = importutils.import_object( + "manila.share.manager.ShareManager") + self.context = context.get_admin_context() + + def _make_share_instance(self, share_id='fake-share-id'): + return {'share_id': share_id, 'id': share_id} + + def _make_export_loc(self, path, preferred=False, is_admin_only=False): + return { + 'path': path, + 'is_admin_only': is_admin_only, + 'metadata': {'preferred': preferred}, + } + + def test_no_dns_metadata_returns_empty(self): + export_locs = [ + self._make_export_loc('10.0.0.1:/share', preferred=True)] + + result = self.share_manager._create_dns_export_locations( + None, None, export_locs) + + self.assertEqual([], result) + + def test_only_dns_name_no_domain_returns_empty(self): + export_locs = [ + self._make_export_loc('10.0.0.1:/share', preferred=True)] + + result = self.share_manager._create_dns_export_locations( + 'myshare', None, export_locs) + + self.assertEqual([], result) + + def test_dns_location_created_for_preferred_export(self): + export_locs = [ + self._make_export_loc('10.0.0.1:/volumes/share1', preferred=True), + ] + + result = self.share_manager._create_dns_export_locations( + 'myshare', 'manila.example.com.', export_locs) + + self.assertEqual(1, len(result)) + self.assertEqual( + 'myshare.manila.example.com:/volumes/share1', result[0]['path']) + self.assertFalse(result[0]['is_admin_only']) + self.assertEqual('myshare', result[0]['metadata']['dns_name']) + self.assertEqual('manila.example.com.', + result[0]['metadata']['dns_domain']) + + def test_non_preferred_export_locations_are_skipped(self): + export_locs = [ + self._make_export_loc('10.0.0.1:/share', preferred=False), + self._make_export_loc('10.0.0.2:/share', preferred=False), + ] + + result = self.share_manager._create_dns_export_locations( + 'myshare', 'manila.example.com.', export_locs) + + self.assertEqual([], result) + + def test_only_preferred_location_gets_dns_path(self): + export_locs = [ + self._make_export_loc('10.0.0.1:/share', preferred=False, + is_admin_only=True), + self._make_export_loc('10.0.0.2:/share', preferred=True), + self._make_export_loc('10.0.0.3:/share', preferred=False), + ] + + result = self.share_manager._create_dns_export_locations( + 'myshare', 'manila.example.com.', export_locs) + + self.assertEqual(1, len(result)) + self.assertEqual( + 'myshare.manila.example.com:/share', result[0]['path']) + + def test_dns_path_is_marked_preferred(self): + export_locs = [ + self._make_export_loc('10.0.0.1:/share', preferred=True), + ] + + result = self.share_manager._create_dns_export_locations( + 'myshare', 'manila.example.com.', export_locs) + + self.assertTrue(result[0]['metadata']['preferred']) + + def test_create_dns_record_not_called_when_disabled(self): + self.mock_object(self.share_manager.dns_api, 'create_record') + share_instance = self._make_share_instance() + export_locs = [ + self._make_export_loc('10.0.0.1:/share', preferred=True)] + + self.share_manager._create_dns_record( + self.context, share_instance, None, None, export_locs) + + self.share_manager.dns_api.create_record.assert_not_called() + + def test_create_dns_record_called_with_preferred_ip(self): + self.mock_object(self.share_manager.dns_api, 'create_record') + self.share_manager.dns_api._enabled = True + self.share_manager.dns_api._zone_id = 'fake-zone-id' + share_instance = self._make_share_instance() + export_locs = [ + self._make_export_loc('10.0.0.1:/share', preferred=False, + is_admin_only=True), + self._make_export_loc('10.0.0.2:/share', preferred=True), + ] + + self.share_manager._create_dns_record( + self.context, share_instance, 'myshare', 'manila.example.com.', + export_locs) + + self.share_manager.dns_api.create_record.assert_called_once_with( + self.context, 'myshare', 'manila.example.com.', ['10.0.0.2']) + + def test_create_dns_record_skipped_when_no_dns_metadata(self): + self.mock_object(self.share_manager.dns_api, 'create_record') + self.share_manager.dns_api._enabled = True + self.share_manager.dns_api._zone_id = 'fake-zone-id' + share_instance = self._make_share_instance() + export_locs = [ + self._make_export_loc('10.0.0.1:/share', preferred=True)] + + self.share_manager._create_dns_record( + self.context, share_instance, None, None, export_locs) + + self.share_manager.dns_api.create_record.assert_not_called() + + def test_delete_dns_record_called_when_metadata_set(self): + self.share_manager.dns_api._enabled = True + self.share_manager.dns_api._zone_id = 'fake-zone-id' + self.mock_object(self.share_manager.dns_api, 'delete_record') + fake_share = {'id': 'fake-share-id'} + self.mock_object(self.share_manager.db, 'share_metadata_get', + mock.Mock(return_value={ + 'dns_name': 'myshare', + 'dns_domain': 'manila.example.com.'})) + + self.share_manager._delete_dns_record(self.context, fake_share) + + self.share_manager.dns_api.delete_record.assert_called_once_with( + self.context, 'myshare', 'manila.example.com.') + + def test_delete_dns_record_not_called_when_no_metadata(self): + self.share_manager.dns_api._enabled = True + self.share_manager.dns_api._zone_id = 'fake-zone-id' + self.mock_object(self.share_manager.dns_api, 'delete_record') + fake_share = {'id': 'fake-share-id'} + self.mock_object(self.share_manager.db, 'share_metadata_get', + mock.Mock(return_value={})) + + self.share_manager._delete_dns_record(self.context, fake_share) + + self.share_manager.dns_api.delete_record.assert_not_called() + + def test_delete_dns_record_not_called_when_disabled(self): + self.mock_object(self.share_manager.dns_api, 'delete_record') + fake_share = {'id': 'fake-share-id'} + + self.share_manager._delete_dns_record(self.context, fake_share) + + self.share_manager.dns_api.delete_record.assert_not_called() diff --git a/releasenotes/notes/add-dns-metadata-for-export-locations-1200e7d5a.yaml b/releasenotes/notes/add-dns-metadata-for-export-locations-1200e7d5a.yaml new file mode 100644 index 0000000000..6803329b1a --- /dev/null +++ b/releasenotes/notes/add-dns-metadata-for-export-locations-1200e7d5a.yaml @@ -0,0 +1,30 @@ +--- +features: + - | + Add DNS metadata support for NFS shares. When a share is created with + dns_name and dns_domain metadata, Manila automatically: + + - Creates a DNS record in OpenStack Designate pointing to the preferred + export location IP. + - Adds an extra export location with the DNS path + alongside the existing IP-based paths, so clients + can mount by name instead of IP. + + This solves the problem of NFS clients needing to update mount + configurations when a share server is recreated and gets a different IP - + the DNS name stays stable even if the underlying IP changes. + +security: + - | + There is no RBAC mechanism in Designate equivalent to Neutron's + device_owner protection for ports. Any user with access to the + Designate zone can modify or delete the A records created by Manila. + + - Manila is not notified of external Designate changes, so the DNS export + location entry in Manila's database will remain even after the record is + deleted in Designate, leading to a stale path. + + Suggestion: + + - To reduce risk, it is recommended to restrict write access to the Designate + zone used for Manila shares to the Manila service account only.