Skip to content

Commit

Permalink
Empty RecordSets can be re-provisioned
Browse files Browse the repository at this point in the history
An empty recordset is just a logical entity with a given record type.
Right now we save the recordset name and type even when it is a null set,
thus preventing us from re-using the name as a different type (eg. A to
CNAME). It would be better to check for this empty recordset condition
and delete if possible, rather than throwing error 400.
This commit also adds CNAME type to Record and RecordSet fixtures
Closes-Bug: #1304088

Change-Id: I4dd0e7def848939d8f0053a5ca40b14a0b68b8b3
  • Loading branch information
Darshan Sanghani committed Jul 26, 2014
1 parent b26f35d commit 1dba323
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 0 deletions.
10 changes: 10 additions & 0 deletions designate/api/v1/records.py
Expand Up @@ -203,6 +203,15 @@ def update_record(domain_id, record_id):
return flask.jsonify(record_schema.filter(record))


def _delete_recordset_if_empty(context, domain_id, recordset_id):
recordset = get_central_api().find_recordset(context, {
'id': recordset_id
})
# Make sure it's the right recordset
if len(recordset.records) == 0:
get_central_api().delete_recordset(context, domain_id, recordset_id)


@blueprint.route('/domains/<uuid:domain_id>/records/<uuid:record_id>',
methods=['DELETE'])
def delete_record(domain_id, record_id):
Expand All @@ -219,4 +228,5 @@ def delete_record(domain_id, record_id):
get_central_api().delete_record(
context, domain_id, record['recordset_id'], record_id)

_delete_recordset_if_empty(context, domain_id, record['recordset_id'])
return flask.Response(status=200)
8 changes: 8 additions & 0 deletions designate/tests/__init__.py
Expand Up @@ -204,6 +204,10 @@ class TestCase(base.BaseTestCase):
{'name': '_sip._tcp.%s', 'type': 'SRV'},
{'name': '_sip._udp.%s', 'type': 'SRV'},
],
'CNAME': [
{'name': 'www.%s', 'type': 'CNAME'},
{'name': 'sub1.%s', 'type': 'CNAME'},
]
}

record_fixtures = {
Expand All @@ -218,6 +222,10 @@ class TestCase(base.BaseTestCase):
'SRV': [
{'data': '0 5060 server1.example.org.', 'priority': 5},
{'data': '1 5060 server2.example.org.', 'priority': 10},
],
'CNAME': [
{'data': 'www.somedomain.org.'},
{'data': 'www.someotherdomain.com.'},
]
}

Expand Down
49 changes: 49 additions & 0 deletions designate/tests/test_api/test_v1/test_records.py
Expand Up @@ -65,6 +65,55 @@ def test_create_record_existing_recordset(self):
self.assertIn('name', response.json)
self.assertEqual(response.json['name'], fixture['name'])

def test_create_record_name_reuse(self):
fixture_1 = self.get_record_fixture(self.recordset['type'])
fixture_1.update({
'name': self.recordset['name'],
'type': self.recordset['type'],
})

fixture_2 = self.get_record_fixture(self.recordset['type'], fixture=1)
fixture_2.update({
'name': self.recordset['name'],
'type': self.recordset['type'],
})

# Create 2 records
record_1 = self.post('domains/%s/records' % self.domain['id'],
data=fixture_1)
record_2 = self.post('domains/%s/records' % self.domain['id'],
data=fixture_2)

# Delete record 1, this should not have any side effects
self.delete('domains/%s/records/%s' % (self.domain['id'],
record_1.json['id']))

# Get the record 2 to ensure recordset did not get deleted
rec_2_get_response = self.get('domains/%s/records/%s' %
(self.domain['id'], record_2.json['id']))

self.assertIn('id', rec_2_get_response.json)
self.assertIn('name', rec_2_get_response.json)
self.assertEqual(rec_2_get_response.json['name'], fixture_1['name'])

# Delete record 2, this should delete the null recordset too
self.delete('domains/%s/records/%s' % (self.domain['id'],
record_2.json['id']))

# Re-create as a different type, but use the same name
fixture = self.get_record_fixture('CNAME')
fixture.update({
'name': self.recordset['name'],
'type': 'CNAME'
})

response = self.post('domains/%s/records' % self.domain['id'],
data=fixture)

self.assertIn('id', response.json)
self.assertIn('name', response.json)
self.assertEqual(response.json['name'], fixture['name'])

def test_create_record_junk(self):
fixture = self.get_record_fixture(self.recordset['type'])
fixture.update({
Expand Down

0 comments on commit 1dba323

Please sign in to comment.