Fix share network update erroneously returns success

This patch fixes an issue while performing share network update.
Manila was allowing the user to update a share network even if it
did not contain a default subnet. Now, we throw an error if there
is no default subnet to be updated. Also, adds an extra validation.
Now, Manila do not allow the default share network subnet update if
it is going to have only `neutron_net_id` or `neutron_subnet_id`
after the update.

Change-Id: Iba2761db53da68a1c497e2e5dd370c36a22ebbed
Closes-Bug: #1846836

manila/api/v2/share_networks.py

@@ -272,10 +272,35 @@ class ShareNetworkController(wsgi.Controller):
                     'neutron_subnet_id' in update_values):
                 subnet = db_api.share_network_subnet_get_default_subnet(
                     context, id)
-                if subnet:
-                    db_api.share_network_subnet_update(context,
-                                                       subnet['id'],
-                                                       update_values)
+                if not subnet:
+                    msg = _("The share network %(id)s does not have a "
+                            "'default' subnet that serves all availability "
+                            "zones, so subnet details "
+                            "('neutron_net_id', 'neutron_subnet_id') cannot "
+                            "be updated.") % {'id': id}
+                    raise exc.HTTPBadRequest(explanation=msg)
+
+                # NOTE(silvacarlose): If the default share network subnet have
+                # the fields neutron_net_id and neutron_subnet_id set as None,
+                # we need to make sure that in the update request the user is
+                # passing both parameter since a share network subnet must
+                # have both fields filled or empty.
+                subnet_neutron_net_and_subnet_id_are_empty = (
+                    subnet['neutron_net_id'] is None
+                    and subnet['neutron_subnet_id'] is None)
+                update_values_without_neutron_net_or_subnet = (
+                    update_values.get('neutron_net_id') is None or
+                    update_values.get('neutron_subnet_id') is None)
+                if (subnet_neutron_net_and_subnet_id_are_empty
+                        and update_values_without_neutron_net_or_subnet):
+                    msg = _(
+                        "To update the share network %(id)s you need to "
+                        "specify both 'neutron_net_id' and "
+                        "'neutron_subnet_id'.") % {'id': id}
+                    raise webob.exc.HTTPBadRequest(explanation=msg)
+                db_api.share_network_subnet_update(context,
+                                                   subnet['id'],
+                                                   update_values)
             share_network = db_api.share_network_update(context,
                                                         id,
                                                         update_values)

manila/tests/api/v2/test_share_networks.py

@@ -813,6 +813,9 @@ class ShareNetworkAPITest(test.TestCase):
         self.mock_object(
             self.controller, '_share_network_subnets_contain_share_servers',
             mock.Mock(return_value=False))
+        self.mock_object(db_api, 'share_network_subnet_get_default_subnet',
+                         mock.Mock(return_value=fake_share_network_subnet))
+        self.mock_object(db_api, 'share_network_subnet_update')
 
         body = {share_networks.RESOURCE_NAME: {'neutron_subnet_id':
                                                'new subnet'}}
@@ -825,6 +828,49 @@ class ShareNetworkAPITest(test.TestCase):
                               self.req,
                               share_nw,
                               body)
+        db_api.share_network_subnet_get_default_subnet.assert_called_once_with(
+            self.context, share_nw)
+        db_api.share_network_subnet_update.assert_called_once_with(
+            self.context, fake_share_network_subnet['id'],
+            body['share_network'])
+
+    @ddt.data((webob_exc.HTTPBadRequest, fake_share_network_subnet, None,
+               'new subnet'),
+              (webob_exc.HTTPBadRequest, None, 'neutron net', None))
+    @ddt.unpack
+    def test_update_default_subnet_errors(self, exception_to_raise,
+                                          get_default_subnet_return,
+                                          neutron_net_id, neutron_subnet_id):
+        share_nw = 'fake network id'
+        self.mock_object(db_api, 'share_network_get',
+                         mock.Mock(return_value=fake_share_network))
+        self.mock_object(
+            self.controller, '_share_network_subnets_contain_share_servers',
+            mock.Mock(return_value=False))
+        self.mock_object(db_api, 'share_network_subnet_get_default_subnet',
+                         mock.Mock(return_value=get_default_subnet_return))
+
+        if get_default_subnet_return:
+            fake_subnet = copy.deepcopy(fake_share_network_subnet)
+            fake_subnet['neutron_net_id'] = None
+            fake_subnet['neutron_subnet_id'] = None
+            db_api.share_network_subnet_get_default_subnet.return_value = (
+                fake_subnet)
+        body = {
+            share_networks.RESOURCE_NAME: {
+                'neutron_net_id': neutron_net_id,
+                'neutron_subnet_id': neutron_subnet_id
+            }
+        }
+
+        self.assertRaises(exception_to_raise,
+                          self.controller.update,
+                          self.req,
+                          share_nw,
+                          body)
+
+        db_api.share_network_subnet_get_default_subnet.assert_called_once_with(
+            self.context, share_nw)
 
     @ddt.data(*set(("1.0", "2.25", "2.26", api_version._MAX_API_VERSION)))
     def test_action_add_security_service(self, microversion):

releasenotes/notes/bug-1846836-fix-share-network-update-unexpected-success-eba8f40db392c467.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixed unexpected behavior when updating a share network's `neutron_net_id`
+    or `neutron_subnet_id`. Now, Manila does not allow updating a share
+    network that does not contain a default subnet.