Hide zones flagged for deletion in API v1

Add docstrings and improve testing
Related to https://bugs.launchpad.net/designate/+bug/1506757

Change-Id: Iaf4c2b28c7f62985c2b4abc0f7d791c91760bfa5
This commit is contained in:
Federico Ceratto 2015-11-18 17:05:20 +00:00
parent c5949ccb28
commit a0ff7859a7
7 changed files with 64 additions and 12 deletions

View File

@ -76,22 +76,27 @@ def create_domain():
@blueprint.route('/domains', methods=['GET'])
def get_domains():
"""List existing zones except those flagged for deletion
"""
context = flask.request.environ.get('context')
central_api = central_rpcapi.CentralAPI.get_instance()
domains = central_api.find_zones(context, criterion={"type": "PRIMARY"})
domains = central_api.find_zones(context, criterion={"type": "PRIMARY",
"action": "!DELETE"})
return flask.jsonify(domains_schema.filter({'domains': domains}))
@blueprint.route('/domains/<uuid:domain_id>', methods=['GET'])
def get_domain(domain_id):
"""Return zone data unless the zone is flagged for purging
"""
context = flask.request.environ.get('context')
central_api = central_rpcapi.CentralAPI.get_instance()
criterion = {"id": domain_id, "type": "PRIMARY"}
criterion = {"id": domain_id, "type": "PRIMARY", "action": "!DELETE"}
domain = central_api.find_zone(context, criterion=criterion)
return flask.jsonify(domain_schema.filter(domain))
@ -105,7 +110,7 @@ def update_domain(domain_id):
central_api = central_rpcapi.CentralAPI.get_instance()
# Fetch the existing resource
criterion = {"id": domain_id, "type": "PRIMARY"}
criterion = {"id": domain_id, "type": "PRIMARY", "action": "!DELETE"}
domain = central_api.find_zone(context, criterion=criterion)
# Prepare a dict of fields for validation
@ -129,7 +134,7 @@ def delete_domain(domain_id):
central_api = central_rpcapi.CentralAPI.get_instance()
# TODO(ekarlso): Fix this to something better.
criterion = {"id": domain_id, "type": "PRIMARY"}
criterion = {"id": domain_id, "type": "PRIMARY", "action": "!DELETE"}
central_api.find_zone(context, criterion=criterion)
central_api.delete_zone(context, domain_id)
@ -144,7 +149,7 @@ def get_domain_servers(domain_id):
central_api = central_rpcapi.CentralAPI.get_instance()
# TODO(ekarlso): Fix this to something better.
criterion = {"id": domain_id, "type": "PRIMARY"}
criterion = {"id": domain_id, "type": "PRIMARY", "action": "!DELETE"}
central_api.find_zone(context, criterion=criterion)
nameservers = central_api.get_zone_ns_records(context, domain_id)

View File

@ -42,7 +42,7 @@ def _find_recordset(context, domain_id, name, type):
def _find_or_create_recordset(context, domain_id, name, type, ttl):
central_api = central_rpcapi.CentralAPI.get_instance()
criterion = {"id": domain_id, "type": "PRIMARY"}
criterion = {"id": domain_id, "type": "PRIMARY", "action": "!DELETE"}
central_api.find_zone(context, criterion=criterion)
try:
@ -191,7 +191,7 @@ def update_record(domain_id, record_id):
# NOTE: We need to ensure the domain actually exists, otherwise we may
# return a record not found instead of a domain not found
criterion = {"id": domain_id, "type": "PRIMARY"}
criterion = {"id": domain_id, "type": "PRIMARY", "action": "!DELETE"}
central_api.find_zone(context, criterion)
# Fetch the existing resource
@ -250,7 +250,7 @@ def delete_record(domain_id, record_id):
# NOTE: We need to ensure the domain actually exists, otherwise we may
# return a record not found instead of a domain not found
criterion = {"id": domain_id, "type": "PRIMARY"}
criterion = {"id": domain_id, "type": "PRIMARY", "action": "!DELETE"}
central_api.find_zone(context, criterion=criterion)
# Find the record

View File

@ -949,6 +949,8 @@ class Service(service.RPCService, service.Service):
return zone
def get_zone(self, context, zone_id):
"""Get a zone, even if flagged for deletion
"""
zone = self.storage.get_zone(context, zone_id)
target = {
@ -987,6 +989,8 @@ class Service(service.RPCService, service.Service):
def find_zones(self, context, criterion=None, marker=None, limit=None,
sort_key=None, sort_dir=None):
"""List existing zones including the ones flagged for deletion.
"""
target = {'tenant_id': context.tenant}
policy.check('find_zones', context, target)

View File

@ -16,6 +16,7 @@
# under the License.
import datetime
import testtools
from mock import patch
import oslo_messaging as messaging
from oslo_config import cfg
@ -235,6 +236,17 @@ class ApiV1zonesTest(ApiV1Test):
del fixture['email']
self.post('domains', data=fixture, status_code=400)
def test_create_zone_twice(self):
self.create_zone()
with testtools.ExpectedException(exceptions.DuplicateZone):
self.create_zone()
def test_create_zone_pending_deletion(self):
zone = self.create_zone()
self.delete('domains/%s' % zone['id'])
with testtools.ExpectedException(exceptions.DuplicateZone):
self.create_zone()
def test_get_zones(self):
response = self.get('domains')
@ -424,6 +436,11 @@ class ApiV1zonesTest(ApiV1Test):
data = {'description': invalid_des}
self.put('domains/%s' % zone['id'], data=data, status_code=400)
def test_update_zone_in_pending_deletion(self):
zone = self.create_zone()
self.delete('domains/%s' % zone['id'])
self.put('domains/%s' % zone['id'], data={}, status_code=404)
def test_delete_zone(self):
# Create a zone
zone = self.create_zone()
@ -439,6 +456,21 @@ class ApiV1zonesTest(ApiV1Test):
# Ensure we can no longer fetch the zone
self.get('domains/%s' % zone['id'], status_code=404)
def test_zone_in_pending_deletion(self):
zone1 = self.create_zone()
self.create_zone(fixture=1)
response = self.get('domains')
self.assertEqual(2, len(response.json['domains']))
# Delete zone1
self.delete('domains/%s' % zone1['id'])
# Ensure we can no longer list nor fetch the deleted zone
response = self.get('domains')
self.assertEqual(1, len(response.json['domains']))
self.get('domains/%s' % zone1['id'], status_code=404)
@patch.object(central_service.Service, 'delete_zone',
side_effect=messaging.MessagingTimeout())
def test_delete_zone_timeout(self, _):

View File

@ -808,7 +808,7 @@ class ApiV1RecordsTest(ApiV1Test):
self.delete('/domains/%s' % self.zone['id'])
self.post('domains/%s/records' % self.zone['id'],
data=fixture, status_code=400)
data=fixture, status_code=404)
def test_update_record_deleting_zone(self):
# Create a record
@ -822,7 +822,7 @@ class ApiV1RecordsTest(ApiV1Test):
self.delete('/domains/%s' % self.zone['id'])
self.put('domains/%s/records/%s' % (self.zone['id'],
record['id']),
data=data, status_code=400)
data=data, status_code=404)
def test_delete_record_deleting_zone(self):
# Create a record
@ -831,4 +831,4 @@ class ApiV1RecordsTest(ApiV1Test):
self.delete('/domains/%s' % self.zone['id'])
self.delete('domains/%s/records/%s' % (self.zone['id'],
record['id']),
status_code=400)
status_code=404)

View File

@ -433,6 +433,10 @@ class ApiV2ZonesTest(ApiV2TestCase):
self.assertEqual('DELETE', response.json['action'])
self.assertEqual('PENDING', response.json['status'])
# The deleted zone should still be listed
zones = self.client.get('/zones/')
self.assertEqual(1, len(zones.json['zones']))
def test_delete_zone_invalid_id(self):
self._assert_invalid_uuid(self.client.delete, '/zones/%s')

View File

@ -2954,12 +2954,19 @@ class CentralServiceTest(CentralTestCase):
# Create a zone
zone = self.create_zone()
# Delete the zone
# Delete the zone (flag it for purging)
self.central_service.delete_zone(self.admin_context, zone['id'])
# The domain should be still there, albeit flagged for purging
self.central_service.get_zone(self.admin_context, zone['id'])
zones = self.central_service.find_zones(self.admin_context)
self.assertEqual(1, len(zones))
# Simulate the zone having been deleted on the backend
zone_serial = self.central_service.get_zone(
self.admin_context, zone['id']).serial
self.central_service.update_status(
self.admin_context, zone['id'], "SUCCESS", zone_serial)