diff options
author | Zuul <zuul@review.opendev.org> | 2022-06-03 07:35:14 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2022-06-03 07:35:14 +0000 |
commit | c6ec485dc841a667ddf5d0733eb92eb51c99a87e (patch) | |
tree | 6c04a353fbc7181802e72be5960f48ef6f4d0784 | |
parent | e0bfba27aa2b24112eec4edf6fa2a0d932838588 (diff) | |
parent | b51c972400bcde8e40d0c7e46110d57cd432fbc2 (diff) | |
download | designate-c6ec485dc841a667ddf5d0733eb92eb51c99a87e.tar.gz |
Merge "Modernize PTR implementation in Central" into stable/wallaby
-rw-r--r-- | designate/central/service.py | 321 | ||||
-rw-r--r-- | designate/tests/test_api/test_v2/test_floatingips.py | 19 | ||||
-rw-r--r-- | designate/tests/test_central/test_service.py | 44 |
3 files changed, 187 insertions, 197 deletions
diff --git a/designate/central/service.py b/designate/central/service.py index 635e1662..cd551a94 100644 --- a/designate/central/service.py +++ b/designate/central/service.py @@ -1616,7 +1616,7 @@ class Service(service.RPCService): # Update the recordset recordset = self.storage.update_recordset(context, recordset) - return (recordset, zone) + return recordset, zone @rpc.expected_exceptions() @notification('dns.recordset.delete') @@ -1765,7 +1765,7 @@ class Service(service.RPCService): record = self.storage.create_record(context, zone.id, recordset.id, record) - return (record, zone) + return record, zone @rpc.expected_exceptions() def get_record(self, context, zone_id, recordset_id, record_id): @@ -1906,7 +1906,7 @@ class Service(service.RPCService): # Update the record record = self.storage.update_record(context, record) - return (record, zone) + return record, zone @rpc.expected_exceptions() @notification('dns.record.delete') @@ -1978,7 +1978,7 @@ class Service(service.RPCService): record = self.storage.update_record(context, record) - return (record, zone) + return record, zone @rpc.expected_exceptions() def count_records(self, context, criterion=None): @@ -2090,16 +2090,15 @@ class Service(service.RPCService): 'storage': storage_status } - def _determine_floatingips(self, context, fips, records=None, - tenant_id=None): + def _determine_floatingips(self, context, fips, project_id=None): """ - Given the context or tenant, records and fips it returns the valid - floatingips either with a associated record or not. Deletes invalid + Given the context or project, and fips it returns the valid + floating ips either with an associated record or not. Deletes invalid records also. - Returns a list of tuples with FloatingIPs and it's Record. + Returns a list of tuples with FloatingIPs and its Record. """ - tenant_id = tenant_id or context.project_id + project_id = project_id or context.project_id elevated_context = context.elevated(all_tenants=True, edit_managed_records=True) @@ -2113,23 +2112,24 @@ class Service(service.RPCService): invalid = [] data = {} - # First populate the list of FIPS + # First populate the list of FIPS. for fip_key, fip_values in fips.items(): # Check if the FIP has a record record = records.get(fip_values['address']) - # NOTE: Now check if it's owned by the tenant that actually has the - # FIP in the external service and if not invalidate it (delete it) - # thus not returning it with in the tuple with the FIP, but None.. + # NOTE: Now check if it's owned by the project that actually has + # the FIP in the external service and if not invalidate it + # (delete it) thus not returning it with in the tuple with the FIP, + # but None. if record: - record_tenant = record['managed_tenant_id'] - - if record_tenant != tenant_id: - msg = "Invalid FloatingIP %s belongs to %s but record " \ - "owner %s" - LOG.debug(msg, fip_key, tenant_id, record_tenant) + record_project = record['managed_tenant_id'] + if record_project != project_id: + LOG.debug( + 'Invalid FloatingIP %s belongs to %s but record ' + 'project %s', fip_key, project_id, record_project + ) invalid.append(record) record = None data[fip_key] = (fip_values, record) @@ -2140,66 +2140,17 @@ class Service(service.RPCService): """ Utility method to delete a list of records. """ + if not records: + return + elevated_context = context.elevated(all_tenants=True, edit_managed_records=True) - if len(records) > 0: - for r in records: - msg = 'Deleting record %s for FIP %s' - LOG.debug(msg, r['id'], r['managed_resource_id']) - self.delete_record(elevated_context, r['zone_id'], - r['recordset_id'], r['id']) - - def _format_floatingips(self, context, data, recordsets=None): - """ - Given a list of FloatingIP and Record tuples we look through creating - a new dict of FloatingIPs - """ - elevated_context = context.elevated(all_tenants=True) - - fips = objects.FloatingIPList() - for key, value in data.items(): - fip, record = value - - fip_ptr = objects.FloatingIP().from_dict({ - 'address': fip['address'], - 'id': fip['id'], - 'region': fip['region'], - 'ptrdname': None, - 'ttl': None, - 'description': None, - 'action': None, - 'status': 'ACTIVE' - }) - - # TTL population requires a present record in order to find the - # RS or Zone - if record: - fip_ptr['action'] = record.action - fip_ptr['status'] = record.status - - # We can have a recordset dict passed in - if (recordsets is not None and - record['recordset_id'] in recordsets): - recordset = recordsets[record['recordset_id']] - else: - recordset = self.storage.get_recordset( - elevated_context, record['recordset_id']) - - if recordset['ttl'] is not None: - fip_ptr['ttl'] = recordset['ttl'] - else: - zone = self.get_zone( - elevated_context, record['zone_id']) - fip_ptr['ttl'] = zone['ttl'] - - fip_ptr['ptrdname'] = record['data'] - fip_ptr['description'] = record['description'] - else: - LOG.debug("No record information found for %s", value[0]['id']) - - # Store the "fip_record" with the region and it's id as key - fips.append(fip_ptr) - return fips + for record in records: + LOG.debug('Deleting record %s for FIP %s', + record['id'], record['managed_resource_id']) + self._delete_ptr_record( + elevated_context, record + ) def _list_floatingips(self, context, region=None): data = self.network_api.list_floatingips(context, region=region) @@ -2216,9 +2167,11 @@ class Service(service.RPCService): def _get_floatingip(self, context, region, floatingip_id, fips): if (region, floatingip_id) not in fips: - msg = 'FloatingIP %s in %s is not associated for tenant "%s"' % \ - (floatingip_id, region, context.project_id) - raise exceptions.NotFound(msg) + raise exceptions.NotFound( + 'FloatingIP %s in %s is not associated for project "%s"' % ( + floatingip_id, region, context.project_id + ) + ) return fips[region, floatingip_id] # PTR ops @@ -2234,14 +2187,15 @@ class Service(service.RPCService): elevated_context = context.elevated(all_tenants=True, edit_managed_records=True) - tenant_fips = self._list_floatingips(context) + project_floatingips = self._list_floatingips(context) valid, invalid = self._determine_floatingips( - elevated_context, tenant_fips) + elevated_context, project_floatingips + ) self._invalidate_floatingips(context, invalid) - return self._format_floatingips(context, valid) + return self._create_floating_ip_list(context, valid) @rpc.expected_exceptions() def get_floatingip(self, context, region, floatingip_id): @@ -2257,11 +2211,12 @@ class Service(service.RPCService): result = self._list_to_dict([fip], keys=['region', 'id']) valid, invalid = self._determine_floatingips( - elevated_context, result) + elevated_context, result + ) self._invalidate_floatingips(context, invalid) - return self._format_floatingips(context, valid)[0] + return self._create_floating_ip_list(context, valid)[0] def _set_floatingip_reverse(self, context, region, floatingip_id, values): """ @@ -2271,16 +2226,18 @@ class Service(service.RPCService): elevated_context = context.elevated(all_tenants=True, edit_managed_records=True) - tenant_fips = self._list_floatingips(context, region=region) + project_fips = self._list_floatingips(context, region=region) - fip = self._get_floatingip(context, region, floatingip_id, tenant_fips) + fip = self._get_floatingip( + context, region, floatingip_id, project_fips + ) zone_name = self.network_api.address_zone(fip['address']) - # NOTE: Find existing zone or create it.. try: zone = self.storage.find_zone( - elevated_context, {'name': zone_name}) + elevated_context, {'name': zone_name} + ) except exceptions.ZoneNotFound: LOG.info( 'Creating zone for %(fip_id)s:%(region)s - %(fip_addr)s ' @@ -2303,44 +2260,16 @@ class Service(service.RPCService): } zone = self.create_zone( - elevated_context, objects.Zone(**zone_values)) + elevated_context, objects.Zone(**zone_values) + ) record_name = self.network_api.address_name(fip['address']) - recordset_values = { 'name': record_name, + 'zone_id': zone['id'], 'type': 'PTR', - 'ttl': values.get('ttl', None) + 'ttl': values.get('ttl') } - - try: - recordset = self.find_recordset( - elevated_context, {'name': record_name, 'type': 'PTR'}) - - # Update the recordset values - recordset.name = recordset_values['name'] - recordset.type = recordset_values['type'] - recordset.ttl = recordset_values['ttl'] - recordset.zone_id = zone['id'] - recordset = self.update_recordset( - elevated_context, - recordset=recordset) - - # Delete the current records for the recordset - LOG.debug("Removing old Record") - for record in recordset.records: - self.delete_record( - elevated_context, - zone_id=recordset['zone_id'], - recordset_id=recordset['id'], - record_id=record['id']) - - except exceptions.RecordSetNotFound: - recordset = self.create_recordset( - elevated_context, - zone_id=zone['id'], - recordset=objects.RecordSet(**recordset_values)) - record_values = { 'data': values['ptrdname'], 'description': values['description'], @@ -2351,16 +2280,14 @@ class Service(service.RPCService): 'managed_resource_type': 'ptr:floatingip', 'managed_tenant_id': context.project_id } - - record = self.create_record( - elevated_context, - zone_id=zone['id'], - recordset_id=recordset['id'], - record=objects.Record(**record_values)) - - return self._format_floatingips( - context, {(region, floatingip_id): (fip, record)}, - {recordset['id']: recordset})[0] + record = objects.Record(**record_values) + recordset = self._replace_or_create_ptr_recordset( + elevated_context, record, + **recordset_values + ) + return self._create_floating_ip( + context, fip, record, zone=zone, recordset=recordset + ) def _unset_floatingip_reverse(self, context, region, floatingip_id): """ @@ -2380,16 +2307,121 @@ class Service(service.RPCService): try: record = self.storage.find_record( - elevated_context, criterion=criterion) + elevated_context, criterion=criterion + ) except exceptions.RecordNotFound: msg = 'No such FloatingIP %s:%s' % (region, floatingip_id) raise exceptions.NotFound(msg) - self.delete_record( - elevated_context, - zone_id=record['zone_id'], - recordset_id=record['recordset_id'], - record_id=record['id']) + self._delete_ptr_record( + elevated_context, record + ) + + def _create_floating_ip(self, context, fip, record, + zone=None, recordset=None): + """ + Creates a FloatingIP based on floating ip and record data. + """ + elevated_context = context.elevated(all_tenants=True) + fip_ptr = objects.FloatingIP().from_dict({ + 'address': fip['address'], + 'id': fip['id'], + 'region': fip['region'], + 'ptrdname': None, + 'ttl': None, + 'description': None, + 'action': None, + 'status': 'ACTIVE' + }) + # TTL population requires a present record in order to find the + # RS or Zone. + if record and record.action != 'DELETE': + if not recordset: + recordset = self.storage.get_recordset( + elevated_context, record.recordset_id) + + fip_ptr['action'] = recordset.action + fip_ptr['status'] = recordset.status + + if recordset.ttl is not None: + fip_ptr['ttl'] = recordset.ttl + else: + if not zone: + zone = self.get_zone(elevated_context, + record.zone_id) + fip_ptr['ttl'] = zone.ttl + + fip_ptr['ptrdname'] = record.data + fip_ptr['description'] = record.description + else: + LOG.debug('No record information found for %s', fip['id']) + + return fip_ptr + + def _create_floating_ip_list(self, context, data): + """ + Creates a FloatingIPList based on floating ips and records data. + """ + fips = objects.FloatingIPList() + for key, value in data.items(): + fip, record = value + fip_ptr = self._create_floating_ip(context, fip, record) + fips.append(fip_ptr) + return fips + + def _delete_ptr_record(self, context, record): + try: + recordset = self.get_recordset( + context, record.zone_id, record.recordset_id + ) + + if record not in recordset.records: + LOG.debug( + 'PTR Record %s not found in recordset %s', + record.id, record.recordset_id + ) + return + + recordset.records.remove(record) + + if not recordset.records: + self.delete_recordset( + context, record.zone_id, record.recordset_id + ) + return + + recordset.validate() + self.update_recordset(context, recordset) + except exceptions.RecordSetNotFound: + pass + + def _replace_or_create_ptr_recordset(self, context, record, zone_id, + name, type, ttl=None): + try: + recordset = self.find_recordset(context, { + 'zone_id': zone_id, + 'name': name, + 'type': type, + }) + recordset.ttl = ttl + recordset.records = objects.RecordList(objects=[record]) + recordset.validate() + recordset = self.update_recordset( + context, recordset + ) + except exceptions.RecordSetNotFound: + values = { + 'name': name, + 'type': type, + 'ttl': ttl, + } + recordset = objects.RecordSet(**values) + recordset.records = objects.RecordList(objects=[record]) + recordset.validate() + recordset = self.create_recordset( + context, zone_id, recordset + ) + return recordset @rpc.expected_exceptions() @transaction @@ -2398,12 +2430,15 @@ class Service(service.RPCService): We strictly see if values['ptrdname'] is str or None and set / unset the requested FloatingIP's PTR record based on that. """ - if 'ptrdname' in values.obj_what_changed() and\ - values['ptrdname'] is None: - self._unset_floatingip_reverse(context, region, floatingip_id) - elif isinstance(values['ptrdname'], six.string_types): + if ('ptrdname' in values.obj_what_changed() and + values['ptrdname'] is None): + self._unset_floatingip_reverse( + context, region, floatingip_id + ) + elif isinstance(values['ptrdname'], str): return self._set_floatingip_reverse( - context, region, floatingip_id, values) + context, region, floatingip_id, values + ) # Blacklisted zones @rpc.expected_exceptions() diff --git a/designate/tests/test_api/test_v2/test_floatingips.py b/designate/tests/test_api/test_v2/test_floatingips.py index 2810029d..4667da55 100644 --- a/designate/tests/test_api/test_v2/test_floatingips.py +++ b/designate/tests/test_api/test_v2/test_floatingips.py @@ -197,19 +197,6 @@ class ApiV2ReverseFloatingIPTest(ApiV2TestCase): self.central_service.update_floatingip( context, fip['region'], fip['id'], fixture) - criterion = { - 'managed_resource_id': fip['id'], - 'managed_tenant_id': context.project_id - } - zone_id = self.central_service.find_record( - elevated_context, criterion=criterion).zone_id - - # Simulate the update on the backend - zone_serial = self.central_service.get_zone( - elevated_context, zone_id).serial - self.central_service.update_status( - elevated_context, zone_id, "SUCCESS", zone_serial) - # Unset PTR ('ptrdname' is None aka null in JSON) response = self.client.patch_json( '/reverse/floatingips/%s' % ":".join([fip['region'], fip['id']]), @@ -218,12 +205,6 @@ class ApiV2ReverseFloatingIPTest(ApiV2TestCase): self.assertIsNone(response.json) self.assertEqual(202, response.status_int) - # Simulate the unset on the backend - zone_serial = self.central_service.get_zone( - elevated_context, zone_id).serial - self.central_service.update_status( - elevated_context, zone_id, "SUCCESS", zone_serial) - fip = self.central_service.get_floatingip( context, fip['region'], fip['id']) self.assertIsNone(fip['ptrdname']) diff --git a/designate/tests/test_central/test_service.py b/designate/tests/test_central/test_service.py index 94a23306..6961e3dd 100644 --- a/designate/tests/test_central/test_service.py +++ b/designate/tests/test_central/test_service.py @@ -2422,8 +2422,10 @@ class CentralServiceTest(CentralTestCase): actual = self.central_service.get_floatingip( context, fip['region'], fip['id']) - self.assertEqual(expected, actual) + self.assertEqual(expected.address, actual.address) + self.assertEqual(expected.ptrdname, actual.ptrdname) + self.assertEqual(expected.ttl, actual.ttl) self.assertEqual(expected, actual) def test_get_floatingip_not_allocated(self): @@ -2456,14 +2458,6 @@ class CentralServiceTest(CentralTestCase): criterion = { 'managed_resource_id': fip['id'], 'managed_tenant_id': context_a.project_id} - zone_id = self.central_service.find_record( - elevated_a, criterion).zone_id - - # Simulate the update on the backend - zone_serial = self.central_service.get_zone( - elevated_a, zone_id).serial - self.central_service.update_status( - elevated_a, zone_id, "SUCCESS", zone_serial) self.network_api.fake.deallocate_floatingip(fip['id']) @@ -2485,19 +2479,9 @@ class CentralServiceTest(CentralTestCase): context_b, fip['region'], fip['id']) self.assertIsNone(fip_ptr['ptrdname']) - # Simulate the invalidation on the backend - zone_serial = self.central_service.get_zone( - elevated_a, zone_id).serial - self.central_service.update_status( - elevated_a, zone_id, "SUCCESS", zone_serial) - - # Ensure that the old record for tenant a for the fip now owned by - # tenant b is gone - exc = self.assertRaises(rpc_dispatcher.ExpectedException, - self.central_service.find_record, - elevated_a, criterion) - - self.assertEqual(exceptions.RecordNotFound, exc.exc_info[0]) + record = self.central_service.find_record(elevated_a, criterion) + self.assertEqual('DELETE', record.action) + self.assertEqual('PENDING', record.status) def test_list_floatingips_no_allocations(self): context = self.get_context(project_id='a') @@ -2583,19 +2567,9 @@ class CentralServiceTest(CentralTestCase): self.assertEqual(1, len(fips)) self.assertIsNone(fips[0]['ptrdname']) - # Simulate the invalidation on the backend - zone_serial = self.central_service.get_zone( - elevated_a, zone_id).serial - self.central_service.update_status( - elevated_a, zone_id, "SUCCESS", zone_serial) - - # Ensure that the old record for tenant a for the fip now owned by - # tenant b is gone - exc = self.assertRaises(rpc_dispatcher.ExpectedException, - self.central_service.find_record, - elevated_a, criterion) - - self.assertEqual(exceptions.RecordNotFound, exc.exc_info[0]) + record = self.central_service.find_record(elevated_a, criterion) + self.assertEqual('DELETE', record.action) + self.assertEqual('PENDING', record.status) def test_set_floatingip(self): context = self.get_context(project_id='a') |