From 61b76377fd41c312cafc0470cffdaee8143507e7 Mon Sep 17 00:00:00 2001 From: Lingxian Kong Date: Tue, 11 Aug 2020 10:51:04 +1200 Subject: [PATCH] Support getting and updating instance access info * Added "public" and "allowed_cidrs" fields for getting instance. * Added '--is-public/--is-private' and '--allowed-cidr' for updating. Change-Id: Ifc919667e83573bd6c3b7aff205fe08e82667457 --- ...ctoria-get-and-update-instance-access.yaml | 3 + troveclient/osc/v1/database_instances.py | 62 ++++++++++++++----- troveclient/tests/fakes.py | 4 +- .../tests/osc/v1/test_database_instances.py | 54 +++++++++++----- troveclient/tests/test_instances.py | 25 +------- troveclient/v1/instances.py | 21 +++++-- 6 files changed, 109 insertions(+), 60 deletions(-) create mode 100644 releasenotes/notes/victoria-get-and-update-instance-access.yaml diff --git a/releasenotes/notes/victoria-get-and-update-instance-access.yaml b/releasenotes/notes/victoria-get-and-update-instance-access.yaml new file mode 100644 index 00000000..18ef3b4f --- /dev/null +++ b/releasenotes/notes/victoria-get-and-update-instance-access.yaml @@ -0,0 +1,3 @@ +--- +features: + - Added support to show and update access settings for instance. diff --git a/troveclient/osc/v1/database_instances.py b/troveclient/osc/v1/database_instances.py index 0b0e7573..9935420b 100644 --- a/troveclient/osc/v1/database_instances.py +++ b/troveclient/osc/v1/database_instances.py @@ -49,11 +49,15 @@ def set_attributes_for_print(instances): instance.datastore['version']) setattr(instance, 'datastore', instance.datastore['type']) + if 'access' in instance_info: + setattr(instance, "public", + instance_info["access"].get("is_public", False)) + return instances def set_attributes_for_print_detail(instance): - info = instance._info.copy() + info = instance.to_dict() info['flavor'] = instance.flavor['id'] if hasattr(instance, 'volume'): info['volume'] = instance.volume['size'] @@ -80,6 +84,11 @@ def set_attributes_for_print_detail(instance): info['fault_date'] = instance.fault['created'] if 'details' in instance.fault and instance.fault['details']: info['fault_details'] = instance.fault['details'] + if hasattr(instance, 'access'): + info['public'] = instance.access["is_public"] + info['allowed_cidrs'] = instance.access["allowed_cidrs"] + info.pop("access", None) + info.pop('links', None) return info @@ -87,11 +96,8 @@ def set_attributes_for_print_detail(instance): class ListDatabaseInstances(command.Lister): _description = _("List database instances") columns = ['ID', 'Name', 'Datastore', 'Datastore Version', 'Status', - 'Addresses', 'Flavor ID', 'Size', 'Region', 'Role'] - admin_columns = [ - 'ID', 'Name', 'Tenant ID', 'Datastore', 'Datastore Version', 'Status', - 'Addresses', 'Flavor ID', 'Size', 'Role' - ] + 'Public', 'Addresses', 'Flavor ID', 'Size', 'Role'] + admin_columns = columns + ["Tenant ID"] def get_parser(self, prog_name): parser = super(ListDatabaseInstances, self).get_parser(prog_name) @@ -346,7 +352,7 @@ class CreateDatabaseInstance(command.ShowOne): action='append', dest='allowed_cidrs', help="The IP CIDRs that are allowed to access the database " - "instance.", + "instance. Repeat for multiple values", ) return parser @@ -664,21 +670,49 @@ class UpdateDatabaseInstance(command.Command): ) parser.add_argument( '--remove_configuration', + '--remove-configuration', dest='remove_configuration', action="store_true", default=False, help=_('Drops the current configuration reference.'), ) + public_group = parser.add_mutually_exclusive_group() + public_group.add_argument( + '--is-public', + dest='public', + default=None, + action='store_true', + help="Make the database instance accessible to public.", + ) + public_group.add_argument( + '--is-private', + dest='public', + default=None, + action='store_false', + help="Make the database instance inaccessible to public.", + ) + parser.add_argument( + '--allowed-cidr', + action='append', + dest='allowed_cidrs', + help="The IP CIDRs that are allowed to access the database " + "instance. Repeat for multiple values", + ) return parser def take_action(self, parsed_args): - db_instances = self.app.client_manager.database.instances - instance = osc_utils.find_resource(db_instances, - parsed_args.instance) - db_instances.edit(instance, parsed_args.configuration, - parsed_args.name, - parsed_args.detach_replica_source, - parsed_args.remove_configuration) + instance_mgr = self.app.client_manager.database.instances + instance_id = parsed_args.instance + + if not uuidutils.is_uuid_like(instance_id): + instance_id = osc_utils.find_resource(instance_mgr, instance_id) + + instance_mgr.update(instance_id, parsed_args.configuration, + parsed_args.name, + parsed_args.detach_replica_source, + parsed_args.remove_configuration, + is_public=parsed_args.public, + allowed_cidrs=parsed_args.allowed_cidrs) class DetachDatabaseInstanceReplica(command.Command): diff --git a/troveclient/tests/fakes.py b/troveclient/tests/fakes.py index 98f44db8..53ea5c5d 100644 --- a/troveclient/tests/fakes.py +++ b/troveclient/tests/fakes.py @@ -175,7 +175,8 @@ class FakeHTTPClient(base_client.HTTPClient): "region": "regionOne", "datastore": {"version": "5.6", "type": "mysql"}, "tenant_id": "fake_tenant_id", - "replica_of": {"id": "fake_master_id"} + "replica_of": {"id": "fake_master_id"}, + "access": {"is_public": False, "allowed_cidrs": []} }, { "id": "5678", @@ -189,6 +190,7 @@ class FakeHTTPClient(base_client.HTTPClient): "region": "regionOne", "datastore": {"version": "5.6", "type": "mysql"}, "tenant_id": "fake_tenant_id", + "access": {"is_public": False, "allowed_cidrs": []} }, ] diff --git a/troveclient/tests/osc/v1/test_database_instances.py b/troveclient/tests/osc/v1/test_database_instances.py index 2801419e..2ffd84d8 100644 --- a/troveclient/tests/osc/v1/test_database_instances.py +++ b/troveclient/tests/osc/v1/test_database_instances.py @@ -55,12 +55,12 @@ class TestInstanceList(TestInstances): ) values = [ - ('1234', 'test-member-1', 'mysql', '5.6', 'ACTIVE', + ('1234', 'test-member-1', 'mysql', '5.6', 'ACTIVE', False, [{"type": "private", "address": "10.0.0.13"}], - '02', 2, 'regionOne', 'replica'), - ('5678', 'test-member-2', 'mysql', '5.6', 'ACTIVE', + '02', 2, 'replica'), + ('5678', 'test-member-2', 'mysql', '5.6', 'ACTIVE', False, [{"type": "private", "address": "10.0.0.14"}], - '2', 2, 'regionOne', '') + '2', 2, '') ] self.assertEqual(values, data) @@ -78,20 +78,20 @@ class TestInstanceList(TestInstances): ) expected_instances = [ - ('1234', 'test-member-1', 'fake_tenant_id', 'mysql', '5.6', - 'ACTIVE', [{"type": "private", "address": "10.0.0.13"}], '02', 2, - 'replica'), - ('5678', 'test-member-2', 'fake_tenant_id', 'mysql', '5.6', - 'ACTIVE', [{"type": "private", "address": "10.0.0.14"}], '2', 2, - '') + ('1234', 'test-member-1', 'mysql', '5.6', + 'ACTIVE', False, [{"type": "private", "address": "10.0.0.13"}], + '02', 2, 'replica', 'fake_tenant_id'), + ('5678', 'test-member-2', 'mysql', '5.6', + 'ACTIVE', False, [{"type": "private", "address": "10.0.0.14"}], + '2', 2, '', 'fake_tenant_id') ] self.assertEqual(expected_instances, instances) class TestInstanceShow(TestInstances): - values = ([{'address': '10.0.0.13', 'type': 'private'}], 'mysql', '5.6', - '02', '1234', 'test-member-1', - 'regionOne', 'fake_master_id', 'ACTIVE', 'fake_tenant_id', 2) + values = ([{'address': '10.0.0.13', 'type': 'private'}], [], 'mysql', + '5.6', '02', '1234', 'test-member-1', False, 'regionOne', + 'fake_master_id', 'ACTIVE', 'fake_tenant_id', 2) def setUp(self): super(TestInstanceShow, self).setUp() @@ -100,11 +100,13 @@ class TestInstanceShow(TestInstances): self.instance_client.get.return_value = self.data self.columns = ( 'addresses', + 'allowed_cidrs', 'datastore', 'datastore_version', 'flavor', 'id', 'name', + 'public', 'region', 'replica_of', 'status', @@ -407,12 +409,30 @@ class TestDatabaseInstanceUpdate(TestInstances): mock_find.return_value = args[0] parsed_args = self.check_parser(self.cmd, args, []) result = self.cmd.take_action(parsed_args) - self.instance_client.edit.assert_called_with('instance1', - None, - 'new_instance_name', - True, True) + self.instance_client.update.assert_called_with( + 'instance1', + None, + 'new_instance_name', + True, True, + is_public=None, allowed_cidrs=None) self.assertIsNone(result) + def test_instance_update_access(self): + ins_id = '4c397f77-750d-43df-8fc5-f7388e4316ee' + args = [ins_id, + '--name', 'new_instance_name', + '--is-private', '--allowed-cidr', '10.0.0.0/24', + '--allowed-cidr', '10.0.1.0/24'] + parsed_args = self.check_parser(self.cmd, args, []) + self.cmd.take_action(parsed_args) + + self.instance_client.update.assert_called_with( + ins_id, + None, + 'new_instance_name', + False, False, + is_public=False, allowed_cidrs=['10.0.0.0/24', '10.0.1.0/24']) + class TestInstanceReplicaDetach(TestInstances): diff --git a/troveclient/tests/test_instances.py b/troveclient/tests/test_instances.py index 24912d0f..1c5f24a2 100644 --- a/troveclient/tests/test_instances.py +++ b/troveclient/tests/test_instances.py @@ -15,9 +15,10 @@ # License for the specific language governing permissions and limitations # under the License. -import testtools from unittest import mock +import testtools + from troveclient import base from troveclient.v1 import instances @@ -223,28 +224,6 @@ class InstancesTest(testtools.TestCase): resp.status_code = 500 self.assertRaises(Exception, self.instances.modify, 'instance1') - def test_edit(self): - resp = mock.Mock() - resp.status_code = 204 - - def fake_patch(url, body): - # Make sure we never pass slave_of to the API. - self.assertIn('instance', body) - self.assertNotIn('slave_of', body['instance']) - return resp, None - - self.instances.api.client.patch = mock.Mock(side_effect=fake_patch) - self.instances.edit(123) - self.instances.edit(123, 321) - self.instances.edit(123, 321, 'name-1234') - self.instances.edit(123, 321, 'name-1234', True) - self.instances.edit(self.instance_with_id) - self.instances.edit(self.instance_with_id, 123) - self.instances.edit(self.instance_with_id, 123, 'name-1234') - self.instances.edit(self.instance_with_id, 123, 'name-1234', True) - resp.status_code = 500 - self.assertRaises(Exception, self.instances.edit, 'instance1') - def test_module_apply(self): resp = mock.Mock() resp.status_code = 200 diff --git a/troveclient/v1/instances.py b/troveclient/v1/instances.py index 2ff17f3b..1a46c10f 100644 --- a/troveclient/v1/instances.py +++ b/troveclient/v1/instances.py @@ -149,11 +149,16 @@ class Instances(base.ManagerWithFind): resp, body = self.api.client.put(url, body=body) common.check_for_exceptions(resp, body, url) - def edit(self, instance, configuration=None, name=None, - detach_replica_source=False, remove_configuration=False): + def update(self, instance, configuration=None, name=None, + detach_replica_source=False, remove_configuration=False, + is_public=None, allowed_cidrs=None): + """Update instance. + + The configuration change, detach_replica and access change cannot be + updated at the same time. + """ body = { - "instance": { - } + "instance": {} } if configuration and remove_configuration: raise Exception("Cannot attach and detach configuration " @@ -166,9 +171,15 @@ class Instances(base.ManagerWithFind): body["instance"]["name"] = name if detach_replica_source: body["instance"]["replica_of"] = None + if is_public is not None or allowed_cidrs is not None: + body["instance"]['access'] = {} + if is_public is not None: + body["instance"]['access']['is_public'] = is_public + if allowed_cidrs is not None: + body["instance"]['access']['allowed_cidrs'] = allowed_cidrs url = "/instances/%s" % base.getid(instance) - resp, body = self.api.client.patch(url, body=body) + resp, body = self.api.client.put(url, body=body) common.check_for_exceptions(resp, body, url) def upgrade(self, instance, datastore_version):