Skip to content

Commit

Permalink
API: change PATCH/PUT on zones to return 204 No Content instead of fu…
Browse files Browse the repository at this point in the history 
…ll zone

Breaks backwards compat, obviously.
  • Loading branch information
@zeha
zeha committed Jun 10, 2016
1 parent 3d6753a commit f0e76ce
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 45 deletions.
4 changes: 3 additions & 1 deletion docs/markdown/httpapi/api_spec.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ REST

* GET: List/Retrieve. Success reply: `200 OK`
* POST: Create. Success reply: `201 Created`, with new object as body.
* PUT: Update. Success reply: `200 OK`, with modified object as body.
* PUT: Update. Success reply: `200 OK`, with modified object as body. For some operations, `204 No Content` is returned instead (and the modified object is not given in the body).
* DELETE: Delete. Success reply: `200 OK`, no body.

not-so-REST
Expand Down Expand Up @@ -467,6 +467,7 @@ Deletes this zone, all attached metadata and rrsets.
#### PATCH

Modifies present RRsets and comments.
Returns `204 No Content` on success.

**Note**: Authoritative only.

Expand Down Expand Up @@ -536,6 +537,7 @@ Client body for PATCH:
Modifies basic zone data (metadata).

Allowed fields in client body: all except `id` and `url`.
Returns `204 No Content` on success.

Changing `name` renames the zone, as expected.

Expand Down
8 changes: 5 additions & 3 deletions pdns/ws-auth.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,8 @@ static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp) {

updateDomainSettingsFromDocument(di, zonename, req->json());

fillZone(zonename, resp);
resp->body = "";
resp->status = 204; // No Content, but indicate success
return;
}
else if(req->method == "DELETE" && !::arg().mustDo("api-readonly")) {
Expand Down Expand Up @@ -1101,8 +1102,9 @@ static void patchZone(HttpRequest* req, HttpResponse* resp) {
// now the PTRs
storeChangedPTRs(B, new_ptrs);

// success
fillZone(zonename, resp);
resp->body = "";
resp->status = 204; // No Content, but indicate success
return;
}

static void apiServerSearchData(HttpRequest* req, HttpResponse* resp) {
Expand Down
3 changes: 2 additions & 1 deletion pdns/ws-recursor.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,8 @@ static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp)
doDeleteZone(zonename);
doCreateZone(document);
reloadAuthAndForwards();
fillZone(apiNameToDNSName(stringFromJson(document, "name")), resp);
resp->body = "";
resp->status = 204; // No Content, but indicate success
}
else if(req->method == "DELETE" && !::arg().mustDo("api-readonly")) {
if (!doDeleteZone(zonename)) {
Expand Down
77 changes: 37 additions & 40 deletions regression-tests.api/test_Zones.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,8 +567,8 @@ def test_update_zone(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
data = r.json()
self.assert_success(r)
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
for k in payload.keys():
self.assertIn(k, data)
self.assertEquals(data[k], payload[k])
Expand All @@ -582,8 +582,8 @@ def test_update_zone(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
data = r.json()
self.assert_success(r)
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
for k in payload.keys():
self.assertIn(k, data)
self.assertEquals(data[k], payload[k])
Expand Down Expand Up @@ -612,10 +612,9 @@ def test_zone_rr_update(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
self.assert_success(r)
# verify that (only) the new record is there
r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
data = r.json()
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
self.assertEquals(get_rrset(data, name, 'NS')['records'], rrset['records'])

def test_zone_rr_update_mx(self):
Expand All @@ -639,10 +638,9 @@ def test_zone_rr_update_mx(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
self.assert_success(r)
# verify that (only) the new record is there
r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
data = r.json()
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
self.assertEquals(get_rrset(data, name, 'MX')['records'], rrset['records'])

def test_zone_rr_update_multiple_rrsets(self):
Expand Down Expand Up @@ -677,10 +675,9 @@ def test_zone_rr_update_multiple_rrsets(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
self.assert_success(r)
# verify that all rrsets have been updated
r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
data = r.json()
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
self.assertEquals(get_rrset(data, name, 'NS')['records'], rrset1['records'])
self.assertEquals(get_rrset(data, name, 'MX')['records'], rrset2['records'])

Expand All @@ -697,10 +694,9 @@ def test_zone_rr_delete(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
self.assert_success(r)
# verify that the records are gone
r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
data = r.json()
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
self.assertIsNone(get_rrset(data, name, 'NS'))

def test_zone_disable_reenable(self):
Expand All @@ -724,9 +720,10 @@ def test_zone_disable_reenable(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
self.assert_success(r)
# check SOA serial has been edited
soa_serial1 = get_first_rec(r.json(), name, 'SOA')['content'].split()[2]
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
soa_serial1 = get_first_rec(data, name, 'SOA')['content'].split()[2]
self.assertNotEquals(soa_serial1, '1')
# make sure domain is still in zone list (disabled SOA!)
r = self.session.get(self.url("/api/v1/servers/localhost/zones"))
Expand All @@ -741,10 +738,10 @@ def test_zone_disable_reenable(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
self.assert_success(r)
# check SOA serial has been edited again
print r.json()
soa_serial2 = get_first_rec(r.json(), name, 'SOA')['content'].split()[2]
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
soa_serial2 = get_first_rec(data, name, 'SOA')['content'].split()[2]
self.assertNotEquals(soa_serial2, '1')
self.assertNotEquals(soa_serial2, soa_serial1)

Expand Down Expand Up @@ -848,7 +845,7 @@ def test_zone_rr_delete_out_of_zone(self):
data=json.dumps(payload),
headers={'content-type': 'application/json'})
print r.content
self.assertEquals(r.status_code, 200) # succeed so users can fix their wrong, old data
self.assert_success(r) # succeed so users can fix their wrong, old data

def test_zone_delete(self):
name, payload, zone = self.create_zone()
Expand Down Expand Up @@ -879,11 +876,11 @@ def test_zone_comment_create(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
self.assert_success(r)
# make sure the comments have been set, and that the NS
# records are still present
r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
serverset = get_rrset(r.json(), name, 'NS')
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
serverset = get_rrset(data, name, 'NS')
print serverset
self.assertNotEquals(serverset['records'], [])
self.assertNotEquals(serverset['comments'], [])
Expand All @@ -904,10 +901,10 @@ def test_zone_comment_delete(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
self.assert_success(r)
# make sure the NS records are still present
r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
serverset = get_rrset(r.json(), name, 'NS')
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
serverset = get_rrset(data, name, 'NS')
print serverset
self.assertNotEquals(serverset['records'], [])
self.assertEquals(serverset['comments'], [])
Expand All @@ -933,7 +930,7 @@ def test_zone_comment_stay_intact(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
self.assert_success(r)
# replace rrset records
rrset2 = {
'changetype': 'replace',
Expand All @@ -952,10 +949,10 @@ def test_zone_comment_stay_intact(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload2),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
self.assert_success(r)
# make sure the comments still exist
r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
serverset = get_rrset(r.json(), name, 'NS')
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
serverset = get_rrset(data, name, 'NS')
print serverset
self.assertEquals(serverset['records'], rrset2['records'])
self.assertEquals(serverset['comments'], rrset['comments'])
Expand Down Expand Up @@ -1015,7 +1012,7 @@ def test_zone_auto_ptr_ipv4_update(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
self.assert_success(r)
r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + revzone)).json()
revsets = [s for s in r['rrsets'] if s['type'] == 'PTR']
print revsets
Expand Down Expand Up @@ -1055,7 +1052,7 @@ def test_zone_auto_ptr_ipv6_update(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
self.assert_success(r)
r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + revzone)).json()
revsets = [s for s in r['rrsets'] if s['type'] == 'PTR']
print revsets
Expand Down Expand Up @@ -1159,8 +1156,8 @@ def test_update_zone(self):
self.url("/api/v1/servers/localhost/zones/" + zone_id),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
data = r.json()
self.assert_success(r)
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + zone_id)).json()
for k in payload.keys():
self.assertIn(k, data)
self.assertEquals(data[k], payload[k])
Expand All @@ -1174,8 +1171,8 @@ def test_update_zone(self):
self.url("/api/v1/servers/localhost/zones/" + zone_id),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
data = r.json()
self.assert_success(r)
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + zone_id)).json()
for k in payload.keys():
self.assertIn(k, data)
self.assertEquals(data[k], payload[k])
Expand Down Expand Up @@ -1257,8 +1254,8 @@ def test_rename_auth_zone(self):
self.url("/api/v1/servers/localhost/zones/" + name),
data=json.dumps(payload),
headers={'content-type': 'application/json'})
self.assert_success_json(r)
data = r.json()
self.assert_success(r)
data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + payload['name'])).json()
for k in payload.keys():
self.assertEquals(data[k], payload[k])

Expand Down
7 changes: 7 additions & 0 deletions regression-tests.api/test_helper.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ def assert_success_json(self, result):
raise
self.assertEquals(result.headers['Content-Type'], 'application/json')

def assert_success(self, result):
try:
result.raise_for_status()
except:
print result.content
raise


def unique_zone_name():
return 'test-' + datetime.now().strftime('%d%H%S%M%f') + '.org.'
Expand Down

0 comments on commit f0e76ce

Please sign in to comment.