From f0e76cee2c83e462ad9350e6772f1aaf10df8e68 Mon Sep 17 00:00:00 2001 From: Christian Hofstaedtler Date: Fri, 10 Jun 2016 14:32:27 +0200 Subject: [PATCH] API: change PATCH/PUT on zones to return 204 No Content instead of full zone Breaks backwards compat, obviously. --- docs/markdown/httpapi/api_spec.md | 4 +- pdns/ws-auth.cc | 8 +-- pdns/ws-recursor.cc | 3 +- regression-tests.api/test_Zones.py | 77 ++++++++++++++--------------- regression-tests.api/test_helper.py | 7 +++ 5 files changed, 54 insertions(+), 45 deletions(-) diff --git a/docs/markdown/httpapi/api_spec.md b/docs/markdown/httpapi/api_spec.md index 9ee46c148222..bdcb78cff14e 100644 --- a/docs/markdown/httpapi/api_spec.md +++ b/docs/markdown/httpapi/api_spec.md @@ -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 @@ -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. @@ -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. diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc index 22f2363487e7..a568c2e5a7d5 100644 --- a/pdns/ws-auth.cc +++ b/pdns/ws-auth.cc @@ -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")) { @@ -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) { diff --git a/pdns/ws-recursor.cc b/pdns/ws-recursor.cc index 72cdf95e824d..6dc2b710960e 100644 --- a/pdns/ws-recursor.cc +++ b/pdns/ws-recursor.cc @@ -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)) { diff --git a/regression-tests.api/test_Zones.py b/regression-tests.api/test_Zones.py index a8cfeb7f0f74..93e345b8c0e7 100644 --- a/regression-tests.api/test_Zones.py +++ b/regression-tests.api/test_Zones.py @@ -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]) @@ -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]) @@ -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): @@ -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): @@ -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']) @@ -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): @@ -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")) @@ -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) @@ -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() @@ -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'], []) @@ -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'], []) @@ -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', @@ -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']) @@ -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 @@ -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 @@ -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]) @@ -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]) @@ -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]) diff --git a/regression-tests.api/test_helper.py b/regression-tests.api/test_helper.py index fc2fdbd34779..04a851a9710c 100644 --- a/regression-tests.api/test_helper.py +++ b/regression-tests.api/test_helper.py @@ -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.'