Support to update instance access

Change-Id: I640cd8b50fd0e0f80a1a45399b8bfdac437ae2b9

12 files changed, 247 insertions, 36 deletions

diff --git a/api-ref/source/instances.inc b/api-ref/source/instances.inc
index 38c38864..1d635fd5 100644
--- a/api-ref/source/instances.inc
+++ b/api-ref/source/instances.inc
@@ -334,7 +334,7 @@ Request Example
 Update instance name
 ~~~~~~~~~~~~~~~~~~~~
 
-.. rest_method::  PATCH /v1.0/{project_id}/instances/{instanceId}
+.. rest_method::  PUT /v1.0/{project_id}/instances/{instanceId}
 
 Update the instance name.
 
@@ -362,7 +362,7 @@ Request Example
 Upgrade datastore version for instance
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-.. rest_method::  PATCH /v1.0/{project_id}/instances/{instanceId}
+.. rest_method::  PUT /v1.0/{project_id}/instances/{instanceId}
 
 Upgrade datastore version.
 
@@ -394,7 +394,7 @@ Request Example
 Detach replica
 ~~~~~~~~~~~~~~
 
-.. rest_method::  PATCH /v1.0/{project_id}/instances/{instanceId}
+.. rest_method::  PUT /v1.0/{project_id}/instances/{instanceId}
 
 Detaches a replica from its replication source.
 
@@ -426,6 +426,38 @@ Request Example
 
 
 
+Update instance accessbility
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. rest_method::  PUT /v1.0/{project_id}/instances/{instanceId}
+
+The following operations are supported:
+
+* If the instance should be exposed to public or not. Not providing
+  ``is_public`` means private.
+* The list of CIDRs that are allowed to access the database service. Not
+  providing ``allowed_cidrs`` means allowing everything.
+
+Normal response codes: 202
+
+Request
+-------
+
+.. rest_parameters:: parameters.yaml
+
+   - project_id: project_id
+   - instanceId: instanceId
+   - instance: instance
+   - access: access
+   - access.is_public: access_is_public
+   - access.allowed_cidrs: access_allowed_cidrs
+
+Request Example
+---------------
+
+.. literalinclude:: samples/instance-update-access-request.json
+   :language: javascript
+
 
 Delete database instance
 ~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/api-ref/source/samples/instance-update-access-request.json b/api-ref/source/samples/instance-update-access-request.json
new file mode 100644
index 00000000..d19f65de
--- /dev/null
+++ b/api-ref/source/samples/instance-update-access-request.json
@@ -0,0 +1,8 @@
+{
+    "instance": {
+        "access": {
+            "is_public": true,
+            "allowed_cidrs": ["10.0.0.0/24"]
+        }
+    }
+}
diff --git a/releasenotes/notes/victoria-show-update-instance-access.yaml b/releasenotes/notes/victoria-show-update-instance-access.yaml
new file mode 100644
index 00000000..d7394162
--- /dev/null
+++ b/releasenotes/notes/victoria-show-update-instance-access.yaml
@@ -0,0 +1,3 @@
+---
+features:
+  - Added support to show and update the access configuration for the instance.
diff --git a/trove/common/apischema.py b/trove/common/apischema.py
index 5b7594f1..2956120a 100644
--- a/trove/common/apischema.py
+++ b/trove/common/apischema.py
@@ -456,6 +456,41 @@ instance = {
             }
         }
     },
+    "update": {
+        "name": "instance:update",
+        "type": "object",
+        "required": ["instance"],
+        "properties": {
+            "instance": {
+                "type": "object",
+                "required": [],
+                "additionalProperties": False,
+                "properties": {
+                    "name": non_empty_string,
+                    "replica_of": {},
+                    "configuration": configuration_id,
+                    "datastore_version": non_empty_string,
+                    "access": {
+                        "type": "object",
+                        "additionalProperties": False,
+                        "properties": {
+                            "is_public": {"type": "boolean"},
+                            "allowed_cidrs": {
+                                "type": "array",
+                                "uniqueItems": True,
+                                "items": {
+                                    "type": "string",
+                                    "pattern": "^([0-9]{1,3}\\.){3}[0-9]{1,3}"
+                                               "(\\/([0-9]|[1-2][0-9]|3[0-2]))"
+                                               "?$"
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    },
     "action": {
         "resize": {
             "volume": {
diff --git a/trove/common/neutron.py b/trove/common/neutron.py
index 5e08a51d..70df7da2 100644
--- a/trove/common/neutron.py
+++ b/trove/common/neutron.py
@@ -77,17 +77,7 @@ def create_port(client, name, description, network_id, security_groups,
     port_id = port['port']['id']
 
     if is_public:
-        public_network_id = get_public_network(client)
-        if not public_network_id:
-            raise exception.PublicNetworkNotFound()
-
-        fip_body = {
-            "floatingip": {
-                'floating_network_id': public_network_id,
-                'port_id': port_id,
-            }
-        }
-        client.create_floatingip(fip_body)
+        make_port_public(client, port_id)
 
     return port_id
 
@@ -107,6 +97,26 @@ def delete_port(client, id):
     client.delete_port(id)
 
 
+def make_port_public(client, port_id):
+    public_network_id = get_public_network(client)
+    if not public_network_id:
+        raise exception.PublicNetworkNotFound()
+
+    fip_body = {
+        "floatingip": {
+            'floating_network_id': public_network_id,
+            'port_id': port_id,
+        }
+    }
+
+    try:
+        client.create_floatingip(fip_body)
+    except Exception as e:
+        LOG.error(f"Failed to associate public IP with port {port_id}: "
+                  f"{str(e)}")
+        raise exception.TroveError('Failed to expose instance port to public.')
+
+
 def get_public_network(client):
     """Get public network ID.
 
@@ -125,6 +135,24 @@ def get_public_network(client):
     return ret['networks'][0].get('id')
 
 
+def ensure_port_access(client, port_id, is_public):
+    fips = client.list_floatingips(port_id=port_id)["floatingips"]
+
+    if is_public and not fips:
+        # Associate floating IP
+        LOG.debug(f"Associate public IP with port {port_id}")
+        make_port_public(client, port_id)
+        return
+
+    if not is_public and fips:
+        # Disassociate floating IP
+        for fip in fips:
+            LOG.debug(f"Disassociate public IP {fip['floating_ip_address']} "
+                      f"from port {port_id}")
+            client.delete_floatingip(fip["id"])
+        return
+
+
 def create_security_group(client, name, instance_id):
     body = {
         'security_group': {
@@ -159,6 +187,15 @@ def create_security_group_rule(client, sg_id, protocol, ports, remote_ips):
             client.create_security_group_rule(body)
 
 
+def clear_ingress_security_group_rules(client, sg_id):
+    rules = client.list_security_group_rules(
+        security_group_id=sg_id)['security_group_rules']
+
+    for rule in rules:
+        if rule['direction'] == 'ingress':
+            client.delete_security_group_rule(rule['id'])
+
+
 def get_subnet_cidrs(client, network_id=None, subnet_id=None):
     cidrs = []
 
diff --git a/trove/instance/models.py b/trove/instance/models.py
index 6875d0b4..bd4814d8 100644
--- a/trove/instance/models.py
+++ b/trove/instance/models.py
@@ -1698,6 +1698,10 @@ class Instance(BuiltInstance):
         self.update_db(task_status=InstanceTasks.BUILDING)
         task_api.API(self.context).rebuild(self.id, image_id)
 
+    def update_access(self, access):
+        self.update_db(task_status=InstanceTasks.UPDATING)
+        task_api.API(self.context).update_access(self.id, access)
+
 
 def create_server_list_matcher(server_list):
     # Returns a method which finds a server from the given list.
diff --git a/trove/instance/service.py b/trove/instance/service.py
index 1ed9ad41..4fae31e7 100644
--- a/trove/instance/service.py
+++ b/trove/instance/service.py
@@ -502,7 +502,9 @@ class InstanceController(wsgi.Controller):
                 context, request=req)
             with StartNotification(context, instance_id=instance.id):
                 instance.detach_replica()
-        if 'configuration_id' in kwargs:
+
+            instance.update_db(**kwargs)
+        elif 'configuration_id' in kwargs:
             if kwargs['configuration_id']:
                 context.notification = (
                     notification.DBaaSInstanceAttachConfiguration(context,
@@ -517,7 +519,9 @@ class InstanceController(wsgi.Controller):
                                                                   request=req))
                 with StartNotification(context, instance_id=instance.id):
                     instance.detach_configuration()
-        if 'datastore_version' in kwargs:
+
+            instance.update_db(**kwargs)
+        elif 'datastore_version' in kwargs:
             datastore_version = ds_models.DatastoreVersion.load(
                 instance.datastore, kwargs['datastore_version'])
             context.notification = (
@@ -525,11 +529,17 @@ class InstanceController(wsgi.Controller):
             with StartNotification(context, instance_id=instance.id,
                                    datastore_version_id=datastore_version.id):
                 instance.upgrade(datastore_version)
-        if kwargs:
+
             instance.update_db(**kwargs)
+        elif 'access' in kwargs:
+            instance.update_access(kwargs['access'])
 
     def update(self, req, id, body, tenant_id):
-        """Updates the instance to attach/detach configuration."""
+        """Updates the instance.
+
+        - attach/detach configuration.
+        - access information.
+        """
         LOG.info("Updating database instance '%(instance_id)s' for tenant "
                  "'%(tenant_id)s'",
                  {'instance_id': id, 'tenant_id': tenant_id})
@@ -537,12 +547,31 @@ class InstanceController(wsgi.Controller):
         LOG.debug("body: %s", body)
         context = req.environ[wsgi.CONTEXT_KEY]
 
+        name = body['instance'].get('name')
+        if ((name and len(body['instance']) != 2) or
+                (not name and len(body['instance']) == 2)):
+            raise exception.BadRequest("Only one attribute (except 'name') is "
+                                       "allowed to update.")
+
         instance = models.Instance.load(context, id)
         self.authorize_instance_action(context, 'update', instance)
 
-        # Make sure args contains a 'configuration_id' argument,
         args = {}
-        args['configuration_id'] = self._configuration_parse(context, body)
+        if name:
+            instance.update_db(name=name)
+
+        detach_replica = ('replica_of' in body['instance'] or
+                          'slave_of' in body['instance'])
+        if detach_replica:
+            args['detach_replica'] = detach_replica
+
+        configuration_id = self._configuration_parse(context, body)
+        if configuration_id:
+            args['configuration_id'] = configuration_id
+
+        if 'access' in body['instance']:
+            args['access'] = body['instance']['access']
+
         self._modify_instance(context, req, instance, **args)
         return wsgi.Result(None, 202)
 
diff --git a/trove/instance/tasks.py b/trove/instance/tasks.py
index 46960fa8..17d699ff 100644
--- a/trove/instance/tasks.py
+++ b/trove/instance/tasks.py
@@ -82,6 +82,7 @@ class InstanceTasks(object):
     LOGGING = InstanceTask(0x0a, 'LOGGING', 'Transferring guest logs.')
     DETACHING = InstanceTask(0x0b, 'DETACHING',
                              'Detaching the instance from replica source.')
+    UPDATING = InstanceTask(0x0c, 'UPDATING', 'Updating the instance.')
 
     BUILDING_ERROR_DNS = InstanceTask(0x50, 'BUILDING', 'Build error: DNS.',
                                       is_error=True)
@@ -121,6 +122,9 @@ class InstanceTasks(object):
     BUILDING_ERROR_PORT = InstanceTask(0x5c, 'BUILDING',
                                        'Build error: Port.',
                                        is_error=True)
+    UPDATING_ERROR_ACCESS = InstanceTask(0x5d, 'UPDATING',
+                                         'Update error: Access.',
+                                         is_error=True)
 
 
 # Dissuade further additions at run-time.
diff --git a/trove/taskmanager/api.py b/trove/taskmanager/api.py
index 61f71f4a..3ffe8d6e 100644
--- a/trove/taskmanager/api.py
+++ b/trove/taskmanager/api.py
@@ -179,6 +179,13 @@ class API(object):
         self._cast("delete_instance", version=version,
                    instance_id=instance_id)
 
+    def update_access(self, instance_id, access):
+        LOG.debug(f"Making async call to update instance: {instance_id}")
+        version = self.API_BASE_VERSION
+
+        self._cast("update_access", version=version,
+                   instance_id=instance_id, access=access)
+
     def create_backup(self, backup_info, instance_id):
         LOG.debug("Making async call to create a backup for instance: %s",
                   instance_id)
diff --git a/trove/taskmanager/manager.py b/trove/taskmanager/manager.py
index 2ac5de87..6259863d 100644
--- a/trove/taskmanager/manager.py
+++ b/trove/taskmanager/manager.py
@@ -458,6 +458,16 @@ class Manager(periodic_task.PeriodicTasks):
         with EndNotification(context):
             instance_tasks.upgrade(datastore_version)
 
+    def update_access(self, context, instance_id, access):
+        instance_tasks = models.BuiltInstanceTasks.load(context, instance_id)
+
+        try:
+            instance_tasks.update_access(access)
+        except Exception as e:
+            LOG.error(f"Failed to update access configuration for "
+                      f"{instance_id}: {str(e)}")
+            self.update_db(task_status=InstanceTasks.UPDATING_ERROR_ACCESS)
+
     def create_cluster(self, context, cluster_id):
         with EndNotification(context, cluster_id=cluster_id):
             cluster_tasks = models.load_cluster_tasks(context, cluster_id)
diff --git a/trove/taskmanager/models.py b/trove/taskmanager/models.py
index 62007445..517e4cf7 100755
--- a/trove/taskmanager/models.py
+++ b/trove/taskmanager/models.py
@@ -1348,6 +1348,60 @@ class BuiltInstanceTasks(BuiltInstance, NotifyMixin, ConfigurationMixin):
         else:
             return "/dev/%s" % device
 
+    def update_access(self, access):
+        LOG.info(f"Updating access for instance {self.id}, access {access}")
+
+        new_is_public = access.get('is_public', False)
+        new_allowed_cidrs = access.get('allowed_cidrs', [])
+        is_public = (self.access.get('is_public', False) if self.access
+                     else None)
+        allowed_cidrs = (self.access.get('allowed_cidrs', []) if self.access
+                         else None)
+
+        ports = self.neutron_client.list_ports(
+            name='trove-%s' % self.id)['ports']
+
+        if is_public != new_is_public:
+            for port in ports:
+                if 'User port' in port['description']:
+                    LOG.debug(f"Updating port {port['id']}, is_public: "
+                              f"{new_is_public}")
+                    neutron.ensure_port_access(self.neutron_client, port['id'],
+                                               new_is_public)
+
+        if CONF.trove_security_groups_support:
+            if allowed_cidrs != new_allowed_cidrs:
+                name = f"{CONF.trove_security_group_name_prefix}-{self.id}"
+                sgs = self.neutron_client.list_security_groups(
+                    name=name)['security_groups']
+
+                LOG.debug(f"Updating security group rules for instance "
+                          f"{self.id}")
+                for sg in sgs:
+                    neutron.clear_ingress_security_group_rules(
+                        self.neutron_client,
+                        sg['id'])
+
+                    if new_allowed_cidrs:
+                        tcp_ports = CONF.get(self.datastore.name).tcp_ports
+                        udp_ports = CONF.get(self.datastore.name).udp_ports
+
+                        neutron.create_security_group_rule(
+                            self.neutron_client, sg['id'], 'tcp', tcp_ports,
+                            new_allowed_cidrs)
+                        neutron.create_security_group_rule(
+                            self.neutron_client, sg['id'], 'udp', udp_ports,
+                            new_allowed_cidrs)
+        else:
+            LOG.warning('Security group not supported.')
+
+        LOG.info(f"Finished to update access for instance {self.id}")
+        self.update_db(
+            task_status=InstanceTasks.NONE,
+            access={'is_public': new_is_public,
+                    'allowed_cidrs': new_allowed_cidrs}
+        )
+
 
 class BackupTasks(object):
     @classmethod
diff --git a/trove/tests/unittests/instance/test_instance_controller.py b/trove/tests/unittests/instance/test_instance_controller.py
index f222ddac..8be487f1 100644
--- a/trove/tests/unittests/instance/test_instance_controller.py
+++ b/trove/tests/unittests/instance/test_instance_controller.py
@@ -304,6 +304,7 @@ class TestInstanceController(trove_testtools.TestCase):
         instance.attach_configuration = Mock()
         instance.detach_configuration = Mock()
         instance.update_db = Mock()
+        instance.update_access = Mock()
         return instance
 
     def test_modify_instance_with_empty_args(self):
@@ -318,16 +319,6 @@ class TestInstanceController(trove_testtools.TestCase):
         self.assertEqual(0, instance.attach_configuration.call_count)
         self.assertEqual(0, instance.update_db.call_count)
 
-    def test_modify_instance_with_nonempty_args_calls_update_db(self):
-        instance = self._setup_modify_instance_mocks()
-        args = {}
-        args['any'] = 'anything'
-
-        self.controller._modify_instance(self.context, self.req,
-                                         instance, **args)
-
-        instance.update_db.assert_called_once_with(**args)
-
     def test_modify_instance_with_False_detach_replica_arg(self):
         instance = self._setup_modify_instance_mocks()
         args = {}
@@ -368,15 +359,12 @@ class TestInstanceController(trove_testtools.TestCase):
 
         self.assertEqual(1, instance.detach_configuration.call_count)
 
-    def test_modify_instance_with_all_args(self):
+    def test_modify_instance_with_access(self):
         instance = self._setup_modify_instance_mocks()
         args = {}
-        args['detach_replica'] = True
-        args['configuration_id'] = 'some_id'
+        args['access'] = {'is_public': True}
 
         self.controller._modify_instance(self.context, self.req,
                                          instance, **args)
 
-        self.assertEqual(1, instance.detach_replica.call_count)
-        self.assertEqual(1, instance.attach_configuration.call_count)
-        instance.update_db.assert_called_once_with(**args)
+        instance.update_access.assert_called_once_with({'is_public': True})