Add "share-network" option for replica create API.

Share replica create API does not allow to specify share network and
forces to use parent share's share network. This is problem for some
use-cases, e.g. migration from one share network to another share
network via replication is not possible. Fixed by allowing to pass
'share-network' option for share replica create API and make sure both
parent share-network and user provided share-network will have same
security service association.

Partial-Bug: #1925486
Change-Id: I9049dcd418fbb16d663ab8ed27b90c765fafc5d3

api-ref/source/share-replicas.inc

@@ -92,11 +92,12 @@ Request example
    :language: javascript
 
 .. note::
-   Since API version 2.51, the parameter ``share_network_id``
-   is deprecated. It will be inherited from its parent share, and the
-   Shared File Systems service will automatically choose which share network
-   subnet your share replica will be placed, according to the specified
-   availability zone.
+   Since API version 2.72, the parameter ``share_network_id`` is added which
+   was earlier supported but later deprecated from version 2.51. In case, the
+   parameter is not specified, it will be inherited from its parent share,
+   and the Shared File Systems service will automatically choose which share
+   network subnet your share replica will be placed, according to the
+   specified availability zone.
 
 
 Response parameters

manila/api/openstack/api_version_request.py

@@ -186,6 +186,7 @@ REST_API_VERSION_HISTORY = """
              availability zone. Also, users can add subnets for an in-use share
              network.
     * 2.71 - Added 'updated_at' field in share instance show API output.
+    * 2.72 - Added new option ``share-network`` to share replica creare API.
 
 """
 
@@ -193,7 +194,7 @@ REST_API_VERSION_HISTORY = """
 # 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.71"
+_MAX_API_VERSION = "2.72"
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 

manila/api/openstack/rest_api_version_history.rst

@@ -396,3 +396,7 @@ ____
 2.71
 ----
   Added 'updated_at' field in share instance show API output.
+
+2.72
+----
+  Added 'share_network' option to share replica create API.

manila/api/v2/share_replicas.py

@@ -21,6 +21,7 @@ import webob
 from webob import exc
 
 from manila.api import common
+from manila.api.openstack import api_version_request as api_version
 from manila.api.openstack import wsgi
 from manila.api.views import share_replicas as replication_view
 from manila.common import constants
@@ -174,11 +175,23 @@ class ShareReplicationController(wsgi.Controller, wsgi.AdminActionsMixin):
                     "since it has been soft deleted.") % share_id
             raise exc.HTTPForbidden(explanation=msg)
 
+        share_network_id = body.get('share_replica').get('share_network_id')
+        if share_network_id:
+            if req.api_version_request < api_version.APIVersionRequest("2.72"):
+                msg = _("'share_network_id' option is not supported by this "
+                        "microversion. Use 2.72 or greater microversion to "
+                        "be able to use 'share_network_id'.")
+                raise exc.HTTPBadRequest(explanation=msg)
+        else:
             share_network_id = share_ref.get('share_network_id', None)
 
+        try:
             if share_network_id:
                 share_network = db.share_network_get(context, share_network_id)
                 common.check_share_network_is_active(share_network)
+        except exception.ShareNetworkNotFound:
+            msg = _("No share network exists with ID %s.")
+            raise exc.HTTPNotFound(explanation=msg % share_network_id)
 
         try:
             new_replica = self.share_api.create_share_replica(

manila/db/api.py

@@ -792,6 +792,12 @@ def security_service_get_all_by_project(context, project_id):
     return IMPL.security_service_get_all_by_project(context, project_id)
 
 
+def security_service_get_all_by_share_network(context, share_network_id):
+    """Get all security service DB records for the given share network."""
+    return IMPL.security_service_get_all_by_share_network(context,
+                                                          share_network_id)
+
+
 ####################
 def share_metadata_get(context, share_id):
     """Get all metadata for a share."""

manila/db/sqlalchemy/api.py

@@ -3977,6 +3977,17 @@ def _security_service_get_query(context, session=None, project_only=False):
                        project_only=project_only)
 
 
+@require_context
+def security_service_get_all_by_share_network(context, share_network_id):
+    session = get_session()
+    return (model_query(context, models.SecurityService, session=session).
+            join(models.ShareNetworkSecurityServiceAssociation,
+            models.SecurityService.id ==
+            models.ShareNetworkSecurityServiceAssociation.security_service_id).
+            filter_by(share_network_id=share_network_id, deleted=0)
+            .all())
+
+
 ###################
 
 

manila/share/api.py

@@ -641,6 +641,21 @@ class API(base.Base):
     def create_share_replica(self, context, share, availability_zone=None,
                              share_network_id=None, scheduler_hints=None):
 
+        parent_share_network_id = share.get('share_network_id')
+        if (parent_share_network_id and share_network_id and
+                parent_share_network_id != share_network_id):
+            parent_share_services = (
+                self.db.security_service_get_all_by_share_network(
+                    context, parent_share_network_id))
+            share_services = (
+                self.db.security_service_get_all_by_share_network(
+                    context, share_network_id))
+            for service in parent_share_services:
+                if service not in share_services:
+                    msg = _("Share and its replica can't be in"
+                            "different authentication domains.")
+                    raise exception.InvalidInput(reason=msg)
+
         if not share.get('replication_type'):
             msg = _("Replication not supported for share %s.")
             raise exception.InvalidShare(message=msg % share['id'])

manila/tests/api/v2/test_share_replicas.py

@@ -396,6 +396,36 @@ class ShareReplicasApiTest(test.TestCase):
         self.mock_policy_check.assert_called_once_with(
             self.member_context, self.resource_name, 'create')
 
+    @ddt.data('2.72')
+    def test_create_invalid_network_id(self, microversion):
+        fake_replica, _ = self._get_fake_replica(
+            replication_type='writable')
+        req = self._get_request(microversion, False)
+        req_context = req.environ['manila.context']
+
+        body = {
+            'share_replica': {
+                'share_id': 'FAKE_SHAREID',
+                'availability_zone': 'FAKE_AZ',
+                'share_network_id': 'FAKE_NETID'
+            }
+        }
+        mock__view_builder_call = self.mock_object(
+            share_replicas.replication_view.ReplicationViewBuilder,
+            'detail_list')
+        self.mock_object(share_replicas.db, 'share_get',
+                         mock.Mock(return_value=fake_replica))
+        self.mock_object(share_replicas.db, 'share_network_get',
+                         mock.Mock(side_effect=exception.ShareNetworkNotFound(
+                                   share_network_id='FAKE_NETID')))
+
+        self.assertRaises(exc.HTTPNotFound,
+                          self.controller.create,
+                          req, body)
+        self.assertFalse(mock__view_builder_call.called)
+        self.mock_policy_check.assert_called_once_with(
+            req_context, self.resource_name, 'create')
+
     @ddt.data(exception.AvailabilityZoneNotFound,
               exception.ReplicationException, exception.ShareBusyException)
     def test_create_exception_path(self, exception_type):
@@ -432,19 +462,25 @@ class ShareReplicasApiTest(test.TestCase):
         common.check_share_network_is_active.assert_called_once_with(
             share_network)
 
-    @ddt.data((True, PRE_GRADUATION_VERSION), (False, GRADUATION_VERSION))
+    @ddt.data((True, PRE_GRADUATION_VERSION), (False, GRADUATION_VERSION),
+              (False, "2.72"))
     @ddt.unpack
     def test_create(self, is_admin, microversion):
         fake_replica, expected_replica = self._get_fake_replica(
             replication_type='writable', admin=is_admin,
             microversion=microversion)
-        share_network = db_utils.create_share_network()
         body = {
             'share_replica': {
                 'share_id': 'FAKE_SHAREID',
                 'availability_zone': 'FAKE_AZ'
             }
         }
+        if self.is_microversion_ge(microversion, '2.72'):
+            body["share_replica"].update({"share_network_id": 'FAKE_NETID'})
+            share_network = {'id': 'FAKE_NETID'}
+        else:
+            share_network = db_utils.create_share_network()
+
         self.mock_object(share_replicas.db, 'share_get',
                          mock.Mock(return_value=fake_replica))
         self.mock_object(share.API, 'create_share_replica',
@@ -465,6 +501,10 @@ class ShareReplicasApiTest(test.TestCase):
         self.assertEqual(expected_replica, res_dict['share_replica'])
         self.mock_policy_check.assert_called_once_with(
             req_context, self.resource_name, 'create')
+        if self.is_microversion_ge(microversion, '2.72'):
+            share_replicas.db.share_network_get.assert_called_once_with(
+                req_context, 'FAKE_NETID')
+        else:
             share_replicas.db.share_network_get.assert_called_once_with(
                 req_context, fake_replica['share_network_id'])
         common.check_share_network_is_active.assert_called_once_with(

manila/tests/db/sqlalchemy/test_api.py

@@ -3121,6 +3121,21 @@ class SecurityServiceDatabaseAPITestCase(BaseDatabaseAPITestCase):
                           self.fake_context,
                           'wrong id')
 
+    def test_get_all_by_share_network(self):
+        db_api.security_service_create(self.fake_context,
+                                       security_service_dict)
+        share_nw_dict = {'id': 'fake network id',
+                         'project_id': 'fake project',
+                         'user_id': 'fake_user_id'}
+        db_api.share_network_create(self.fake_context, share_nw_dict)
+        db_api.share_network_add_security_service(
+            self.fake_context,
+            share_nw_dict['id'], security_service_dict['id'])
+
+        result = db_api.security_service_get_all_by_share_network(
+            self.fake_context, share_nw_dict['id'])
+        self._check_expected_fields(result[0], security_service_dict)
+
     def test_delete(self):
         db_api.security_service_create(self.fake_context,
                                        security_service_dict)

releasenotes/notes/bug-1925486-add-share-network-option-to-replica-create-api-7d2ff3628e93fc77.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    `Bug #1925486 <https://bugs.launchpad.net/manila/+bug/1925486>`_
+    Share replica create API does not support share network option and uses
+    parent share's share network. Fixed it to allow any share network by providing
+    option ``share-network``. Added in API microversion starting with '2.72'.