From: Christian Hofstaedtler Date: Tue, 8 Apr 2014 11:18:28 +0000 (+0200) Subject: API: Replace PATCH zone/rrset API with PATCH zone X-Git-Tag: rec-3.6.0-rc1~72^2~4 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d708640f0e27bf77500de906cfd648b4b31e49ec;p=thirdparty%2Fpdns.git API: Replace PATCH zone/rrset API with PATCH zone PATCH zone updates multiple RRsets at once (and in one transaction). --- diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc index f71cfb09d5..23580774bc 100644 --- a/pdns/ws-auth.cc +++ b/pdns/ws-auth.cc @@ -368,7 +368,7 @@ void productServerStatisticsFetch(map& out) out["uptime"] = lexical_cast(time(0) - s_starttime); } -static void apiServerZoneRRset(HttpRequest* req, HttpResponse* resp); +static void patchZone(HttpRequest* req, HttpResponse* resp); static void apiServerZones(HttpRequest* req, HttpResponse* resp) { UeberBackend B; @@ -528,7 +528,7 @@ static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp) { resp->body = ""; return; } else if (req->method == "PATCH" && !::arg().mustDo("experimental-api-readonly")) { - apiServerZoneRRset(req, resp); + patchZone(req, resp); return; } else if (req->method == "GET") { fillZone(zonename, resp); @@ -576,156 +576,166 @@ static void makePtr(const DNSResourceRecord& rr, DNSResourceRecord* ptr) { ptr->content = rr.qname; } -static void apiServerZoneRRset(HttpRequest* req, HttpResponse* resp) { - if(req->method != "PATCH" || ::arg().mustDo("experimental-api-readonly")) - throw HttpMethodNotAllowedException(); - +static void patchZone(HttpRequest* req, HttpResponse* resp) { UeberBackend B; DomainInfo di; string zonename = apiZoneIdToName(req->path_parameters["id"]); - if(!B.getDomainInfo(zonename, di)) + if (!B.getDomainInfo(zonename, di)) throw ApiException("Could not find domain '"+zonename+"'"); + string dotsuffix = "." + zonename; + vector new_ptrs; + Document document; req->json(document); - string qname, changetype; - QType qtype; - qname = stringFromJson(document, "name"); - qtype = stringFromJson(document, "type"); - changetype = toUpper(stringFromJson(document, "changetype")); + const Value& rrsets = document["rrsets"]; + if (!rrsets.IsArray()) + throw ApiException("No rrsets given in update request"); - string dotsuffix = "." + zonename; - if(!iends_with(qname, dotsuffix) && qname != zonename) - throw ApiException("RRset "+qname+" IN "+qtype.getName()+": Name is out of zone"); - - if (changetype == "DELETE") { - // delete all matching qname/qtype RRs (and, implictly comments). - if (!di.backend->replaceRRSet(di.id, qname, qtype, vector())) { - throw ApiException("Hosting backend does not support editing records."); - } - } - else if (changetype == "REPLACE") { - vector new_records; - vector new_comments; - vector new_ptrs; - bool replace_records = false; - bool replace_comments = false; + di.backend->startTransaction(zonename); - // gather records - DNSResourceRecord rr; - const Value& records = document["records"]; - if (records.IsArray()) { - replace_records = true; - for(SizeType idx = 0; idx < records.Size(); ++idx) { - const Value& record = records[idx]; - rr.qname = stringFromJson(record, "name"); - rr.content = stringFromJson(record, "content"); - rr.qtype = stringFromJson(record, "type"); - rr.domain_id = di.id; - rr.auth = 1; - rr.ttl = intFromJson(record, "ttl"); - rr.priority = intFromJson(record, "priority"); - rr.disabled = boolFromJson(record, "disabled"); - - if(rr.qname != qname || rr.qtype != qtype) - throw ApiException("Record "+rr.qname+" IN "+rr.qtype.getName()+" "+rr.content+": Record bundled with wrong RRset"); - - string temp_content = rr.content; - if(rr.qtype.getCode() == QType::MX || rr.qtype.getCode() == QType::SRV) - temp_content = lexical_cast(rr.priority)+" "+rr.content; - - try { - shared_ptr drc(DNSRecordContent::mastermake(rr.qtype.getCode(), 1, temp_content)); - string tmp = drc->serialize(rr.qname); + try { + for(SizeType rrsetIdx = 0; rrsetIdx < rrsets.Size(); ++rrsetIdx) { + const Value& rrset = rrsets[rrsetIdx]; + string qname, changetype; + QType qtype; + qname = stringFromJson(rrset, "name"); + qtype = stringFromJson(rrset, "type"); + changetype = toUpper(stringFromJson(rrset, "changetype")); + + if (!iends_with(qname, dotsuffix) && qname != zonename) + throw ApiException("RRset "+qname+" IN "+qtype.getName()+": Name is out of zone"); + + if (changetype == "DELETE") { + // delete all matching qname/qtype RRs (and, implictly comments). + if (!di.backend->replaceRRSet(di.id, qname, qtype, vector())) { + throw ApiException("Hosting backend does not support editing records."); } - catch(std::exception& e) - { - throw ApiException("Record "+rr.qname+" IN "+rr.qtype.getName()+" "+rr.content+": "+e.what()); + } + else if (changetype == "REPLACE") { + vector new_records; + vector new_comments; + bool replace_records = false; + bool replace_comments = false; + + // gather records + DNSResourceRecord rr; + const Value& records = rrset["records"]; + if (records.IsArray()) { + replace_records = true; + for (SizeType idx = 0; idx < records.Size(); ++idx) { + const Value& record = records[idx]; + rr.qname = stringFromJson(record, "name"); + rr.content = stringFromJson(record, "content"); + rr.qtype = stringFromJson(record, "type"); + rr.domain_id = di.id; + rr.auth = 1; + rr.ttl = intFromJson(record, "ttl"); + rr.priority = intFromJson(record, "priority"); + rr.disabled = boolFromJson(record, "disabled"); + + if (rr.qname != qname || rr.qtype != qtype) + throw ApiException("Record "+rr.qname+"/"+rr.qtype.getName()+" "+rr.content+": Record wrongly bundled with RRset " + qname + "/" + qtype.getName()); + + string temp_content = rr.content; + if (rr.qtype.getCode() == QType::MX || rr.qtype.getCode() == QType::SRV) + temp_content = lexical_cast(rr.priority)+" "+rr.content; + + try { + shared_ptr drc(DNSRecordContent::mastermake(rr.qtype.getCode(), 1, temp_content)); + string tmp = drc->serialize(rr.qname); + } + catch(std::exception& e) + { + throw ApiException("Record "+rr.qname+"/"+rr.qtype.getName()+" "+rr.content+": "+e.what()); + } + + if ((rr.qtype.getCode() == QType::A || rr.qtype.getCode() == QType::AAAA) && + boolFromJson(record, "set-ptr", false) == true) { + DNSResourceRecord ptr; + makePtr(rr, &ptr); + + // verify that there's a zone for the PTR + DNSPacket fakePacket; + SOAData sd; + fakePacket.qtype = QType::PTR; + if (!B.getAuth(&fakePacket, &sd, ptr.qname, 0)) + throw ApiException("Could not find domain for PTR '"+ptr.qname+"' requested for '"+ptr.content+"'"); + + ptr.domain_id = sd.domain_id; + new_ptrs.push_back(ptr); + } + + new_records.push_back(rr); + } } - if ((rr.qtype.getCode() == QType::A || rr.qtype.getCode() == QType::AAAA) && - boolFromJson(record, "set-ptr", false) == true) { - DNSResourceRecord ptr; - makePtr(rr, &ptr); - - // verify that there's a zone for the PTR - DNSPacket fakePacket; - SOAData sd; - fakePacket.qtype = QType::PTR; - if (!B.getAuth(&fakePacket, &sd, ptr.qname, 0)) - throw ApiException("Could not find domain for PTR '"+ptr.qname+"' requested for '"+ptr.content+"'"); - - ptr.domain_id = sd.domain_id; - new_ptrs.push_back(ptr); + // gather comments + Comment c; + c.domain_id = di.id; + c.qname = qname; + c.qtype = qtype; + time_t now = time(0); + const Value& comments = rrset["comments"]; + if (comments.IsArray()) { + replace_comments = true; + for(SizeType idx = 0; idx < comments.Size(); ++idx) { + const Value& comment = comments[idx]; + c.modified_at = intFromJson(comment, "modified_at", now); + c.content = stringFromJson(comment, "content"); + c.account = stringFromJson(comment, "account"); + new_comments.push_back(c); + } } - new_records.push_back(rr); - } - } + if (!replace_records && !replace_comments) { + throw ApiException("No change for RRset " + qname + "/" + qtype.getName()); + } - // gather comments - Comment c; - c.domain_id = di.id; - c.qname = qname; - c.qtype = qtype; - time_t now = time(0); - const Value& comments = document["comments"]; - if (comments.IsArray()) { - replace_comments = true; - for(SizeType idx = 0; idx < comments.Size(); ++idx) { - const Value& comment = comments[idx]; - c.modified_at = intFromJson(comment, "modified_at", now); - c.content = stringFromJson(comment, "content"); - c.account = stringFromJson(comment, "account"); - new_comments.push_back(c); + if (replace_records) { + if (!di.backend->replaceRRSet(di.id, qname, qtype, new_records)) { + throw ApiException("Hosting backend does not support editing records."); + } + } + if (replace_comments) { + if (!di.backend->replaceComments(di.id, qname, qtype, new_comments)) { + throw ApiException("Hosting backend does not support editing comments."); + } + } } + else + throw ApiException("Changetype not understood"); } + } catch(...) { + di.backend->abortTransaction(); + throw; + } + di.backend->commitTransaction(); - if (!replace_records && !replace_comments) { - throw ApiException("No change"); - } - - // Actually store the change(s). - di.backend->startTransaction(qname); - if (replace_records) { - if (!di.backend->replaceRRSet(di.id, qname, qtype, new_records)) { - throw ApiException("Hosting backend does not support editing records."); - } - } - if (replace_comments) { - if (!di.backend->replaceComments(di.id, qname, qtype, new_comments)) { - throw ApiException("Hosting backend does not support editing comments."); - } - } - di.backend->commitTransaction(); + extern PacketCache PC; + PC.purge(zonename); - // now the PTRs - BOOST_FOREACH(const DNSResourceRecord& rr, new_ptrs) { - DNSPacket fakePacket; - SOAData sd; - sd.db = (DNSBackend *)-1; - fakePacket.qtype = QType::PTR; + // now the PTRs + BOOST_FOREACH(const DNSResourceRecord& rr, new_ptrs) { + DNSPacket fakePacket; + SOAData sd; + sd.db = (DNSBackend *)-1; + fakePacket.qtype = QType::PTR; - if (!B.getAuth(&fakePacket, &sd, rr.qname, 0)) - throw ApiException("Could not find domain for PTR '"+rr.qname+"' requested for '"+rr.content+"' (while saving)"); + if (!B.getAuth(&fakePacket, &sd, rr.qname, 0)) + throw ApiException("Could not find domain for PTR '"+rr.qname+"' requested for '"+rr.content+"' (while saving)"); - sd.db->startTransaction(rr.qname); - if (!sd.db->replaceRRSet(sd.domain_id, rr.qname, rr.qtype, vector(1, rr))) { - throw ApiException("PTR-Hosting backend does not support editing records."); - } - sd.db->commitTransaction(); + sd.db->startTransaction(rr.qname); + if (!sd.db->replaceRRSet(sd.domain_id, rr.qname, rr.qtype, vector(1, rr))) { + throw ApiException("PTR-Hosting backend for "+rr.qname+"/"+rr.qtype.getName()+" does not support editing records."); } - + sd.db->commitTransaction(); + PC.purge(rr.qname); } - else - throw ApiException("Changetype not understood"); - - extern PacketCache PC; - PC.purge(qname); // success - resp->body = "{}"; + fillZone(zonename, resp); } static void apiServerSearchData(HttpRequest* req, HttpResponse* resp) { @@ -912,7 +922,6 @@ void AuthWebServer::webThread() d_ws->registerApiHandler("/servers/localhost/search-log", &apiServerSearchLog); d_ws->registerApiHandler("/servers/localhost/search-data", &apiServerSearchData); d_ws->registerApiHandler("/servers/localhost/statistics", &apiServerStatistics); - d_ws->registerApiHandler("/servers/localhost/zones//rrset", &apiServerZoneRRset); d_ws->registerApiHandler("/servers/localhost/zones/", &apiServerZoneDetail); d_ws->registerApiHandler("/servers/localhost/zones", &apiServerZones); d_ws->registerApiHandler("/servers/localhost", &apiServerDetail); diff --git a/regression-tests.api/test_Zones.py b/regression-tests.api/test_Zones.py index 075534c1ea..587b2ea354 100644 --- a/regression-tests.api/test_Zones.py +++ b/regression-tests.api/test_Zones.py @@ -122,7 +122,7 @@ class AuthZones(ApiTestCase): payload, zone = self.create_zone() name = payload['name'] # do a replace (= update) - payload = { + rrset = { 'changetype': 'replace', 'name': name, 'type': 'NS', @@ -145,23 +145,24 @@ class AuthZones(ApiTestCase): } ] } + payload = {'rrsets': [rrset]} r = self.session.patch( - self.url("/servers/localhost/zones/" + name + "/rrset"), + self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertSuccessJson(r) # verify that (only) the new record is there r = self.session.get(self.url("/servers/localhost/zones/" + name)) data = r.json()['records'] - recs = [rec for rec in data if rec['type'] == payload['type'] and rec['name'] == payload['name']] - self.assertEquals(recs, payload['records']) + recs = [rec for rec in data if rec['type'] == rrset['type'] and rec['name'] == rrset['name']] + self.assertEquals(recs, rrset['records']) def test_ZoneRRUpdateMX(self): # Important to test with MX records, as they have a priority field, which must not end up in the content field. payload, zone = self.create_zone() name = payload['name'] # do a replace (= update) - payload = { + rrset = { 'changetype': 'replace', 'name': name, 'type': 'MX', @@ -176,42 +177,91 @@ class AuthZones(ApiTestCase): } ] } + payload = {'rrsets': [rrset]} r = self.session.patch( - self.url("/servers/localhost/zones/" + name + "/rrset"), + self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertSuccessJson(r) # verify that (only) the new record is there r = self.session.get(self.url("/servers/localhost/zones/" + name)) data = r.json()['records'] - recs = [rec for rec in data if rec['type'] == payload['type'] and rec['name'] == payload['name']] - self.assertEquals(recs, payload['records']) + recs = [rec for rec in data if rec['type'] == rrset['type'] and rec['name'] == rrset['name']] + self.assertEquals(recs, rrset['records']) + + def test_ZoneRRUpdateMultipleRRsets(self): + payload, zone = self.create_zone() + name = payload['name'] + rrset1 = { + 'changetype': 'replace', + 'name': name, + 'type': 'NS', + 'records': [ + { + "name": name, + "type": "NS", + "priority": 0, + "ttl": 3600, + "content": "ns9999.example.com", + "disabled": False + } + ] + } + rrset2 = { + 'changetype': 'replace', + 'name': name, + 'type': 'MX', + 'records': [ + { + "name": name, + "type": "MX", + "priority": 10, + "ttl": 3600, + "content": "mx444.example.com", + "disabled": False + } + ] + } + payload = {'rrsets': [rrset1, rrset2]} + r = self.session.patch( + self.url("/servers/localhost/zones/" + name), + data=json.dumps(payload), + headers={'content-type': 'application/json'}) + self.assertSuccessJson(r) + # verify that all rrsets have been updated + r = self.session.get(self.url("/servers/localhost/zones/" + name)) + data = r.json()['records'] + recs1 = [rec for rec in data if rec['type'] == rrset1['type'] and rec['name'] == rrset1['name']] + self.assertEquals(recs1, rrset1['records']) + recs2 = [rec for rec in data if rec['type'] == rrset2['type'] and rec['name'] == rrset2['name']] + self.assertEquals(recs2, rrset2['records']) def test_ZoneRRDelete(self): payload, zone = self.create_zone() name = payload['name'] # do a delete of all NS records (these are created with the zone) - payload = { + rrset = { 'changetype': 'delete', 'name': name, 'type': 'NS' } + payload = {'rrsets': [rrset]} r = self.session.patch( - self.url("/servers/localhost/zones/" + name + "/rrset"), + self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertSuccessJson(r) # verify that the records are gone r = self.session.get(self.url("/servers/localhost/zones/" + name)) data = r.json()['records'] - recs = [rec for rec in data if rec['type'] == payload['type'] and rec['name'] == payload['name']] + recs = [rec for rec in data if rec['type'] == rrset['type'] and rec['name'] == rrset['name']] self.assertEquals(recs, []) def test_ZoneDisableReenable(self): payload, zone = self.create_zone() name = payload['name'] # disable zone by disabling SOA - payload = { + rrset = { 'changetype': 'replace', 'name': name, 'type': 'SOA', @@ -226,8 +276,9 @@ class AuthZones(ApiTestCase): } ] } + payload = {'rrsets': [rrset]} r = self.session.patch( - self.url("/servers/localhost/zones/" + name + "/rrset"), + self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertSuccessJson(r) @@ -236,9 +287,10 @@ class AuthZones(ApiTestCase): domains = r.json() self.assertEquals(len([domain for domain in domains if domain['name'] == name]), 1) # verify that modifying it still works - payload['records'][0]['disabled'] = False + rrset['records'][0]['disabled'] = False + payload = {'rrsets': [rrset]} r = self.session.patch( - self.url("/servers/localhost/zones/" + name + "/rrset"), + self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertSuccessJson(r) @@ -247,7 +299,7 @@ class AuthZones(ApiTestCase): payload, zone = self.create_zone() name = payload['name'] # replace with qtype mismatch - payload = { + rrset = { 'changetype': 'replace', 'name': name, 'type': 'A', @@ -262,8 +314,9 @@ class AuthZones(ApiTestCase): } ] } + payload = {'rrsets': [rrset]} r = self.session.patch( - self.url("/servers/localhost/zones/" + name + "/rrset"), + self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertEquals(r.status_code, 422) @@ -272,7 +325,7 @@ class AuthZones(ApiTestCase): payload, zone = self.create_zone() name = payload['name'] # replace with qname mismatch - payload = { + rrset = { 'changetype': 'replace', 'name': name, 'type': 'NS', @@ -287,8 +340,9 @@ class AuthZones(ApiTestCase): } ] } + payload = {'rrsets': [rrset]} r = self.session.patch( - self.url("/servers/localhost/zones/" + name + "/rrset"), + self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertEquals(r.status_code, 422) @@ -297,7 +351,7 @@ class AuthZones(ApiTestCase): payload, zone = self.create_zone() name = payload['name'] # replace with qname mismatch - payload = { + rrset = { 'changetype': 'replace', 'name': 'not-in-zone', 'type': 'NS', @@ -312,8 +366,9 @@ class AuthZones(ApiTestCase): } ] } + payload = {'rrsets': [rrset]} r = self.session.patch( - self.url("/servers/localhost/zones/" + name + "/rrset"), + self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertEquals(r.status_code, 422) @@ -323,13 +378,14 @@ class AuthZones(ApiTestCase): payload, zone = self.create_zone() name = payload['name'] # replace with qname mismatch - payload = { + rrset = { 'changetype': 'delete', 'name': 'not-in-zone', 'type': 'NS' } + payload = {'rrsets': [rrset]} r = self.session.patch( - self.url("/servers/localhost/zones/" + name + "/rrset"), + self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertEquals(r.status_code, 422) @@ -338,7 +394,7 @@ class AuthZones(ApiTestCase): def test_ZoneCommentCreate(self): payload, zone = self.create_zone() name = payload['name'] - payload = { + rrset = { 'changetype': 'replace', 'name': name, 'type': 'NS', @@ -353,6 +409,7 @@ class AuthZones(ApiTestCase): } ] } + payload = {'rrsets': [rrset]} r = self.session.patch( self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), @@ -372,12 +429,13 @@ class AuthZones(ApiTestCase): # Test: Delete ONLY comments. payload, zone = self.create_zone() name = payload['name'] - payload = { + rrset = { 'changetype': 'replace', 'name': name, 'type': 'NS', 'comments': [] } + payload = {'rrsets': [rrset]} r = self.session.patch( self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), @@ -395,7 +453,7 @@ class AuthZones(ApiTestCase): payload, zone = self.create_zone() name = payload['name'] # create a comment - payload = { + rrset = { 'changetype': 'replace', 'name': name, 'type': 'NS', @@ -409,13 +467,14 @@ class AuthZones(ApiTestCase): } ] } + payload = {'rrsets': [rrset]} r = self.session.patch( self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertSuccessJson(r) # replace rrset records - payload2 = { + rrset2 = { 'changetype': 'replace', 'name': name, 'type': 'NS', @@ -430,6 +489,7 @@ class AuthZones(ApiTestCase): } ] } + payload2 = {'rrsets': [rrset2]} r = self.session.patch( self.url("/servers/localhost/zones/" + name), data=json.dumps(payload2), @@ -439,8 +499,8 @@ class AuthZones(ApiTestCase): r = self.session.get(self.url("/servers/localhost/zones/" + name)) data = r.json() print data - self.assertEquals([r for r in data['records'] if r['type'] == 'NS'], payload2['records']) - self.assertEquals(data['comments'], payload['comments']) + self.assertEquals([r for r in data['records'] if r['type'] == 'NS'], rrset2['records']) + self.assertEquals(data['comments'], rrset['comments']) def test_ZoneAutoPtrIPv4(self): revzone = '0.2.192.in-addr.arpa' @@ -448,7 +508,7 @@ class AuthZones(ApiTestCase): payload, zone = self.create_zone() name = payload['name'] # replace with qname mismatch - payload = { + rrset = { 'changetype': 'replace', 'name': name, 'type': 'A', @@ -464,8 +524,9 @@ class AuthZones(ApiTestCase): } ] } + payload = {'rrsets': [rrset]} r = self.session.patch( - self.url("/servers/localhost/zones/" + name + "/rrset"), + self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertSuccessJson(r) @@ -489,7 +550,7 @@ class AuthZones(ApiTestCase): payload, zone = self.create_zone() name = payload['name'] # replace with qname mismatch - payload = { + rrset = { 'changetype': 'replace', 'name': name, 'type': 'AAAA', @@ -505,8 +566,9 @@ class AuthZones(ApiTestCase): } ] } + payload = {'rrsets': [rrset]} r = self.session.patch( - self.url("/servers/localhost/zones/" + name + "/rrset"), + self.url("/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertSuccessJson(r)